Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CA-144169: Removing unneeded XAPI calls for VBD.plug operation. #285

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

CA-144169: Removing unneeded XAPI calls for VBD.plug operation. #285

wants to merge 1 commit into from

Conversation

pritha-srivastava
Copy link

CA-144169: Fix to remove XAPI calls to improve time taken by VBD plug.

Signed-off-by: Pritha Srivastava [email protected]

@germanop
Copy link
Contributor

@pritha-srivastava, how much is the performance improvement here? Most of the expensive calls
are still there.

The changes are undoubtedly an improvement and the patch is fine (apart from some style "violations") but we need to understand what is their impact. My educated guess is: negligible.
But I really hope you will prove me wrong :-)

@pritha-srivastava
Copy link
Author

@germanop - The VBD.plug call usually takes under 2 seconds to complete and these changes saved a few milliseconds. I am not sure what the impact would have been when I would have tested that for a 16 host bootstorm case (I couldn't because XenRT didnt allow me to)
Can you point me to what calls have been left behind. I have tried to take things that have been passed by XAPI in the command (device_config) and make use of them, but then most of the times I found that the XAPI calls can not be eliminated (for e.g. finding out the master of the pool).

@germanop
Copy link
Contributor

In function from_uuid:

  • sr_ref = _SR.get_by_uuid(sr_uuid)
  • sm_type = _SR.get_type(sr_ref)
  • sms = _SM.get_all_records_where('field "type" = "%s"' % sm_type)
  • pbds = _PBD.get_all_records_where('field "SR" = "%s" and' % sr_ref +
    'field "host" = "%s"' % host_ref)

There are others in the other functions

@pritha-srivastava
Copy link
Author

@germanop: I have updated the commit message. Let me know if there are other changes to be done.

germanop
germanop reviewed Nov 30, 2015
View reviewed changes
drivers/blktap2.py Outdated
sm_config = self._session.xenapi.VDI.get_sm_config(vdi_ref)
attached_as = util.attached_as(sm_config)
if NO_MULTIPLE_ATTACH and (attached_as == "RW" or \
(attached_as == "RO" and attach_mode == "RW")):
util.SMlog("need to reset VDI %s" % vdi_uuid)
if not resetvdis.reset_vdi(self._session, vdi_uuid, force=False,
term_output=False, writable=writable):
term_output=False, writable=writable, vdiref=vdi_ref,
srref=host_ref, smconf=sm_config):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe sr_ref?

@pritha-srivastava
Copy link
Author

@germanop : All the review comments have been incorporated

@pritha-srivastava pritha-srivastava changed the title Ca 144169 [Work In Progress]CA-144169: Removing unneeded XAPI calls for VBD.plug operation. Dec 21, 2015
@pritha-srivastava pritha-srivastava changed the title [Work In Progress]CA-144169: Removing unneeded XAPI calls for VBD.plug operation. CA-144169: Removing unneeded XAPI calls for VBD.plug operation. Dec 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pritha-srivastava @germanop