Federico Simoncelli has posted comments on this change.

Change subject: vm: discover volume path from xml definition
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.ovirt.org/#/c/24202/1//COMMIT_MSG
Commit Message:

Line 22:  domain
Line 23: 
Line 24: In this patch:
Line 25: 
Line 26: - revert to the previous path for virtual machine images
> Is this really necessary? could you explain why? I think that the former ch
If it was intentional it should have been explicitly stated in the commit 
message so we could have discussed it before reaching this issue. Patch was:

 http://gerrit.ovirt.org/19825

There's nothing backward compatible in changing the image path.

More over if you want run independently from the pool the correct way to do it 
is sending spUUID=BLANK_UUID (as introduced by the same patch).

I will add a note on why we should revert.
Line 27: - inspect libvirt xml during live migration and vdsm restart to
Line 28:   identify if it is necessary to update the path cached in the
Line 29:   drive object (provided by prepareImage)
Line 30: 


http://gerrit.ovirt.org/#/c/24202/1/vdsm/vm.py
File vdsm/vm.py:

Line 4980:                     d.address = address
Line 4981:                     d.readonly = readonly
Line 4982:                     if bootOrder:
Line 4983:                         d.bootOrder = bootOrder
Line 4984:                     self.log.debug('Matched %s', 
mergeDicts(deviceDict, d))
> imo, the previous added block would have been slightly more readable on an 
That would have been wrong, point of updating the path is to check if we're 
able to match it at 4975.
Line 4985:             # Update vm's conf with address for known disk devices
Line 4986:             knownDev = False
Line 4987:             for dev in self.conf['devices']:
Line 4988:                 # See comment in previous loop. This part is used to 
update


-- 
To view, visit http://gerrit.ovirt.org/24202
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I322f1f879fbd5b6415789f3b307e8741d846d694
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Eduardo <ewars...@redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Martin Polednik <mpole...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to