Change in vdsm[master]: v2v: Detect VM with snapshots
Dan Kenigsberg has submitted this change and it was merged. Change subject: v2v: Detect VM with snapshots .. v2v: Detect VM with snapshots virt-v2v cannot properly handle conversion of VMware machine with snapshots. Engine needs a way to know which machines have snapshots and which do not to filter them out and/or notify the user. Note: The snapshot related API is not yet implemented in libvirt Xen driver. It is missing in all libvirt version to date (v1.3.4). Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Signed-off-by: Tomáš Golembiovský Reviewed-on: https://gerrit.ovirt.org/56574 Continuous-Integration: Jenkins CI Reviewed-by: Vinzenz Feenstra Reviewed-by: Francesco Romani --- M lib/api/vdsm-api.yml M lib/api/vdsmapi-schema.json M lib/vdsm/v2v.py M tests/v2vTests.py 4 files changed, 46 insertions(+), 7 deletions(-) Approvals: Jenkins CI: Passed CI tests Vinzenz Feenstra: Looks good to me, but someone else must approve Francesco Romani: Looks good to me, approved Tomas Golembiovsky: Verified -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Francesco Romani has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 10: (1 comment) https://gerrit.ovirt.org/#/c/56574/10/lib/vdsm/v2v.py File lib/vdsm/v2v.py: Line 899: Line 900: Line 901: def _add_snapshot_info(conn, vm, params): Line 902: # Snapshot related API is not yet implemented in the libvirt's Xen driver Line 903: if conn.getType() == 'Xen': > For some reasons I understood we somehow care about the connection driver b BTW, Tomas, since verification could be non-trivial I'm OK if you make the error-handling generic in a followup patch, so we can have some progress. Let's hear from other maintainers. Line 904: return Line 905: Line 906: try: Line 907: ret = vm.hasCurrentSnapshot() -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Francesco Romani has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 10: (1 comment) https://gerrit.ovirt.org/#/c/56574/10/lib/vdsm/v2v.py File lib/vdsm/v2v.py: Line 899: Line 900: Line 901: def _add_snapshot_info(conn, vm, params): Line 902: # Snapshot related API is not yet implemented in the libvirt's Xen driver Line 903: if conn.getType() == 'Xen': > It is not about the possibility of implementing it in the future, but about For some reasons I understood we somehow care about the connection driver being used. If we don't actually care, and we just want to avoid useless scary stacktrace in the logs, then the VIR_ERR_NO_SUPPORT way seems better. Line 904: return Line 905: Line 906: try: Line 907: ret = vm.hasCurrentSnapshot() -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Nir Soffer has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 10: (1 comment) https://gerrit.ovirt.org/#/c/56574/10/lib/vdsm/v2v.py File lib/vdsm/v2v.py: Line 899: Line 900: Line 901: def _add_snapshot_info(conn, vm, params): Line 902: # Snapshot related API is not yet implemented in the libvirt's Xen driver Line 903: if conn.getType() == 'Xen': > Yes, I know. For listAllDomains() it makes sense to catch the exception, be It is not about the possibility of implementing it in the future, but about not making assumptions about libvirt when we can use a generic way to handle unsupported apis. Francesco, what do you think? Line 904: return Line 905: Line 906: try: Line 907: ret = vm.hasCurrentSnapshot() -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Nir Soffer has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 11: It is not about the possibility of implementing it in the future, but about not making assumptions about libvirt when we can use a generic way to handle unsupported apis. -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Tomas Golembiovsky has posted comments on this change.
Change subject: v2v: Detect VM with snapshots
..
Patch Set 11:
(1 comment)
https://gerrit.ovirt.org/#/c/56574/10/lib/vdsm/v2v.py
File lib/vdsm/v2v.py:
Line 899:
Line 900: e = root.find('./vcpu')
Line 901: if e is not None:
Line 902: try:
Line 903: params['smp'] = int(e.text)
> How about trying to acess libvirt, and catching the libvirt.VIR_ERR_NO_SUPP
Yes, I know. For listAllDomains() it makes sense to catch the exception,
because we have an alternative way to get the info. In this case we don't. It
would also make sense if we expected the snapshot related API to be filled in
at some point in the future. But since NONE of the snapshot related API calls
has been implemented in Xen driver so far, I think it's rather questionable it
will ever (or in near future) be.
Line 904: except ValueError:
Line 905: raise InvalidVMConfiguration("Invalid 'vcpu' value: %r" %
e.text)
Line 906:
Line 907: e = root.find('./os/type/[@arch]')
--
To view, visit https://gerrit.ovirt.org/56574
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Golembiovsky
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik
Gerrit-Reviewer: Milan Zamazal
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Piotr Kliczewski
Gerrit-Reviewer: Shahar Havivi
Gerrit-Reviewer: Tomas Golembiovsky
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Nir Soffer has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 10: (1 comment) https://gerrit.ovirt.org/#/c/56574/10/lib/vdsm/v2v.py File lib/vdsm/v2v.py: Line 899: Line 900: Line 901: def _add_snapshot_info(conn, vm, params): Line 902: # Snapshot related API is not yet implemented in the libvirt's Xen driver Line 903: if conn.getType() == 'Xen': > not a fan of this but I can't really think of a better alternative. let it How about trying to acess libvirt, and catching the libvirt.VIR_ERR_NO_SUPPORT error? We use this for listAllDomains() and blockCopy() Line 904: return Line 905: Line 906: try: Line 907: ret = vm.hasCurrentSnapshot() -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Francesco Romani has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 11: Code-Review+2 -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Vinzenz Feenstra has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 11: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Vinzenz Feenstra has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 10: (1 comment) https://gerrit.ovirt.org/#/c/56574/10/lib/vdsm/v2v.py File lib/vdsm/v2v.py: PS10, Line 909: ') > Why don't we care about exc_info? logging.exception does log the exc_info, thats what the difference between error and exception -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Tomas Golembiovsky has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 11: Verified+1 -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Tomas Golembiovsky has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 10: (3 comments) https://gerrit.ovirt.org/#/c/56574/10/lib/api/vdsm-api.yml File lib/api/vdsm-api.yml: PS10, Line 963: Whether the VM has snapshots or not. > I'd prefer different wording, something like (took from below)~ Done Line 964: defaultvalue: null Line 965: name: has_snapshots Line 966: type: boolean Line 967: added: '4.0' Line 968: > This extra line is not needed. Done Line 969: type: object Line 970: Line 971: FcHba: &FcHba Line 972: added: '3.1' https://gerrit.ovirt.org/#/c/56574/10/lib/vdsm/v2v.py File lib/vdsm/v2v.py: PS10, Line 909: ') > Why don't we care about exc_info? The logging module will take care of that and write a proper stack trace to the log. -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Shahar Havivi has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 10: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Martin Polednik has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 10: Code-Review+1 (2 comments) minor Qs/comments inside https://gerrit.ovirt.org/#/c/56574/10/lib/api/vdsm-api.yml File lib/api/vdsm-api.yml: PS10, Line 963: Whether the VM has snapshots or not. I'd prefer different wording, something like (took from below)~ Information on whether VM includes snapshot. https://gerrit.ovirt.org/#/c/56574/10/lib/vdsm/v2v.py File lib/vdsm/v2v.py: PS10, Line 909: ') Why don't we care about exc_info? -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Piotr Kliczewski has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 10: Code-Review+1 (1 comment) Api change looks good. https://gerrit.ovirt.org/#/c/56574/10/lib/api/vdsm-api.yml File lib/api/vdsm-api.yml: Line 964: defaultvalue: null Line 965: name: has_snapshots Line 966: type: boolean Line 967: added: '4.0' Line 968: This extra line is not needed. Line 969: type: object Line 970: Line 971: FcHba: &FcHba Line 972: added: '3.1' -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Francesco Romani has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 10: Code-Review+2 (1 comment) https://gerrit.ovirt.org/#/c/56574/10/lib/vdsm/v2v.py File lib/vdsm/v2v.py: PS10, Line 903: if conn.getType() == 'Xen': not a fan of this but I can't really think of a better alternative. let it be. -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
gerrit-hooks has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 10: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
gerrit-hooks has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 9: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Shahar Havivi has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 8: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
gerrit-hooks has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 8: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
gerrit-hooks has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 7: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Tomas Golembiovsky has posted comments on this change.
Change subject: v2v: Detect VM with snapshots
..
Patch Set 6:
(1 comment)
https://gerrit.ovirt.org/#/c/56574/6/lib/vdsm/v2v.py
File lib/vdsm/v2v.py:
PS6, Line 908: except libvirt.libvirtError as e:
: logging.error('Error checking for existing snapshots;
%r', e.message)
> I prefer if you keep using logging.exception() here.
Done
--
To view, visit https://gerrit.ovirt.org/56574
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Golembiovsky
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik
Gerrit-Reviewer: Milan Zamazal
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Shahar Havivi
Gerrit-Reviewer: Tomas Golembiovsky
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Francesco Romani has posted comments on this change.
Change subject: v2v: Detect VM with snapshots
..
Patch Set 6: Code-Review+1
(1 comment)
https://gerrit.ovirt.org/#/c/56574/6/lib/vdsm/v2v.py
File lib/vdsm/v2v.py:
PS6, Line 908: except libvirt.libvirtError as e:
: logging.error('Error checking for existing snapshots;
%r', e.message)
I prefer if you keep using logging.exception() here.
If we filter out known-not-working scenarios, that will dump only real
meaningful stacktraces.
--
To view, visit https://gerrit.ovirt.org/56574
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Golembiovsky
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik
Gerrit-Reviewer: Milan Zamazal
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Shahar Havivi
Gerrit-Reviewer: Tomas Golembiovsky
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Shahar Havivi has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 6: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Tomas Golembiovsky has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 6: Verified+1 -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
gerrit-hooks has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 6: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Shahar Havivi has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 5: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Francesco Romani has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 5: I'm a bit surprised this is not working on RHEL5. Just for my understanding, you mean the API is not available or does it not work properly (perhaps it always reports no snapshots even if there are some?) -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Francesco Romani has posted comments on this change.
Change subject: v2v: Detect VM with snapshots
..
Patch Set 4:
(1 comment)
https://gerrit.ovirt.org/#/c/56574/4/lib/vdsm/v2v.py
File lib/vdsm/v2v.py:
Line 897: i['model'] = model.get('type')
Line 898: params['networks'].append(i)
Line 899:
Line 900:
Line 901: def _add_has_snapshots(vm, params):
> Renamed to _add_snapshot_info. Hope it's ok.
It is OK for me.
Line 902: try:
Line 903: ret = vm.hasCurrentSnapshot()
Line 904: except libvirt.libvirtError:
Line 905: logging.exception("Error checking for existing snapshots")
--
To view, visit https://gerrit.ovirt.org/56574
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Golembiovsky
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik
Gerrit-Reviewer: Milan Zamazal
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Shahar Havivi
Gerrit-Reviewer: Tomas Golembiovsky
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Tomas Golembiovsky has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 5: Verified-1 The API is not supported in libvirt+Xen on RHEL5. Wi will have to make the check for VMware only. -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Nir Soffer has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 5: Code-Review+1 Waiting for Francesco ack. -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
gerrit-hooks has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 5: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Tomas Golembiovsky has posted comments on this change.
Change subject: v2v: Detect VM with snapshots
..
Patch Set 4:
(2 comments)
https://gerrit.ovirt.org/#/c/56574/4/lib/vdsm/v2v.py
File lib/vdsm/v2v.py:
Line 897: i['model'] = model.get('type')
Line 898: params['networks'].append(i)
Line 899:
Line 900:
Line 901: def _add_has_snapshots(vm, params):
> I would name this _add_snapshots_info to be consistent with other names her
Renamed to _add_snapshot_info. Hope it's ok.
Line 902: try:
Line 903: ret = vm.hasCurrentSnapshot()
Line 904: except libvirt.libvirtError:
Line 905: logging.exception("Error checking for existing snapshots")
https://gerrit.ovirt.org/#/c/56574/4/tests/v2vTests.py
File tests/v2vTests.py:
Line 69: def __init__(self, name="RHEL",
Line 70: vm_uuid="564d7cb4-8e3d-06ec-ce82-7b2b13c6a611",
Line 71: id=0,
Line 72: active=False,
Line 73: snapshots=False):
> Naming this has_snapshot will make it clear that this is a boolean.
Done
Line 74: self._name = name
Line 75: self._uuid = vm_uuid
Line 76: self._mac_address = _mac_from_uuid(vm_uuid)
Line 77: self._id = id
--
To view, visit https://gerrit.ovirt.org/56574
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Golembiovsky
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik
Gerrit-Reviewer: Milan Zamazal
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Shahar Havivi
Gerrit-Reviewer: Tomas Golembiovsky
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Shahar Havivi has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 4: we need to make sure that Libvirt API this is working on rhel 5.x which importing Libvirt Xen to vdsm -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Shahar Havivi Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Nir Soffer has posted comments on this change.
Change subject: v2v: Detect VM with snapshots
..
Patch Set 4:
(2 comments)
https://gerrit.ovirt.org/#/c/56574/4/lib/vdsm/v2v.py
File lib/vdsm/v2v.py:
Line 897: i['model'] = model.get('type')
Line 898: params['networks'].append(i)
Line 899:
Line 900:
Line 901: def _add_has_snapshots(vm, params):
> reads a little awkward, but I don't have better suggestions.
I would name this _add_snapshots_info to be consistent with other names here.
Line 902: try:
Line 903: ret = vm.hasCurrentSnapshot()
Line 904: except libvirt.libvirtError:
Line 905: logging.exception("Error checking for existing snapshots")
https://gerrit.ovirt.org/#/c/56574/4/tests/v2vTests.py
File tests/v2vTests.py:
Line 69: def __init__(self, name="RHEL",
Line 70: vm_uuid="564d7cb4-8e3d-06ec-ce82-7b2b13c6a611",
Line 71: id=0,
Line 72: active=False,
Line 73: snapshots=False):
Naming this has_snapshot will make it clear that this is a boolean.
Line 74: self._name = name
Line 75: self._uuid = vm_uuid
Line 76: self._mac_address = _mac_from_uuid(vm_uuid)
Line 77: self._id = id
--
To view, visit https://gerrit.ovirt.org/56574
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Golembiovsky
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik
Gerrit-Reviewer: Milan Zamazal
Gerrit-Reviewer: Nir Soffer
Gerrit-Reviewer: Tomas Golembiovsky
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Nir Soffer has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 4: (1 comment) https://gerrit.ovirt.org/#/c/56574/1/tests/v2vTests.py File tests/v2vTests.py: Line 75: self._uuid = vm_uuid Line 76: self._mac_address = _mac_from_uuid(vm_uuid) Line 77: self._id = id Line 78: self._active = active Line 79: self._snapshots = snapshots Lets call it has_snapshots to make clear that this is a boolean Line 80: Line 81: def name(self): Line 82: return self._name Line 83: -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Tomas Golembiovsky has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 4: Verified+1 Verified with vdsClient on testing vCenter -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Francesco Romani has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 4: Code-Review+2 (1 comment) https://gerrit.ovirt.org/#/c/56574/4/lib/vdsm/v2v.py File lib/vdsm/v2v.py: PS4, Line 901: _add_has_snapshots reads a little awkward, but I don't have better suggestions. "_add_snapshots" feels way worse, so let's keep it this way. -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
gerrit-hooks has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 4: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Tomas Golembiovsky has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 3: (1 comment) https://gerrit.ovirt.org/#/c/56574/3/tests/v2vTests.py File tests/v2vTests.py: PS3, Line 383: if methodname == 'lookupByName': : # Only active domains : self.assertEqual(len(vms), : len([x for x in VM_SPECS if x.active])) : else: # if methodname == 'lookupByID' : # Only inactive domains : self.assertEqual(len(vms), : len([x for x in VM_SPECS if not x.active])) > for this you'll also need to add another attribute to permutations above: I like the idea. Will do. -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Francesco Romani has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 3: (1 comment) https://gerrit.ovirt.org/#/c/56574/3/tests/v2vTests.py File tests/v2vTests.py: PS3, Line 383: if methodname == 'lookupByName': : # Only active domains : self.assertEqual(len(vms), : len([x for x in VM_SPECS if x.active])) : else: # if methodname == 'lookupByID' : # Only inactive domains : self.assertEqual(len(vms), : len([x for x in VM_SPECS if not x.active])) > Let's make it more explicit and generic: please change it to something like for this you'll also need to add another attribute to permutations above: @permutations([ # (methodname, fakemethod, active) ['lookupByName', lookupByNameFailure, True], ['lookupByID', lookupByIDFailure, False] ]) (perhaps "expected_active" is even better? as you prefer). -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Francesco Romani has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 3: Code-Review-1 (2 comments) OK, let's split the test improvements from this patch, and please make this patch depend on those improvements. -1 for visibility. https://gerrit.ovirt.org/#/c/56574/3/tests/v2vTests.py File tests/v2vTests.py: PS3, Line 383: if methodname == 'lookupByName': : # Only active domains : self.assertEqual(len(vms), : len([x for x in VM_SPECS if x.active])) : else: # if methodname == 'lookupByID' : # Only inactive domains : self.assertEqual(len(vms), : len([x for x in VM_SPECS if not x.active])) Let's make it more explicit and generic: please change it to something like: self.assertEqual( sorted(vm['vmName'] for vm in vms), sorted(spec.name for spec in VM_SPECS if spec.active == active) ) PS3, Line 529: len([x for x in VM_SPECS if not x.active])) same as above, I believe this is more explicit: self.assertEqual(sorted(vms), sorted(spec.name for spec in VM_SPECS if not spec.active)) -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Milan Zamazal has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 3: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
gerrit-hooks has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 3: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
gerrit-hooks has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 2: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Martin Polednik has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 1: Code-Review-1 (3 comments) Minor things + the v2v test could really be made nicer (as suggested by yourself). https://gerrit.ovirt.org/#/c/56574/1//COMMIT_MSG Commit Message: PS1, Line 9: v f https://gerrit.ovirt.org/#/c/56574/1/lib/api/vdsmapi-schema.json File lib/api/vdsmapi-schema.json: PS1, Line 4037: * Marked as optional without #optional in the description, that doesn't seem right. https://gerrit.ovirt.org/#/c/56574/1/lib/vdsm/v2v.py File lib/vdsm/v2v.py: PS1, Line 901: _add_has_snapshots The name could be improved, this wording is a bit awkward. _add_snapshot_flag (don't think that is an improvement though)? -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Tomas Golembiovsky has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 1: (2 comments) https://gerrit.ovirt.org/#/c/56574/1/tests/v2vTests.py File tests/v2vTests.py: PS1, Line 383: if methodname == 'lookupByName': : # Only active domains : self.assertEqual(len(vms), 2) : else: # if methodname == 'lookupByID' : # Only inactive domains : self.assertEqual(len(vms), 3) > why is this needed? As I added another domain into VM_SPECS list this no longer worked. As far as I understand this test it used to work, because the number of active and inactive domains in the list was the same. This is no longer true and the result now differs based on which method is overloaded. However I now see I didn't finish this change. I wanted to use the len() as below in MockVirConnectTest.test_list_defined_domains(). PS1, Line 526: self.assertEqual(len(vms), : len([x for x in VM_SPECS if not x.active])) > why is this needed? Again, as I added another domain into VM_SPEC list this no longer worked and had to be changed from 2 to 3. But I don't think that using some magic constants is a good idea so I replaced it completly with the len(...) that counts the active domains in the list. -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Golembiovsky Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Milan Zamazal has posted comments on this change.
Change subject: v2v: Detect VM with snapshots
..
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/56574/1/lib/vdsm/v2v.py
File lib/vdsm/v2v.py:
Line 899:
Line 900:
Line 901: def _add_has_snapshots(vm, params):
Line 902: try:
Line 903: ret = vm.hasCurrentSnapshot()
> Does this method exist? I can see it only in tests.
Ah, found, it's a libvirt object, sorry.
Line 904: except libvirt.libvirtError:
Line 905: logging.exception("Error checking for existing snapshots")
Line 906: else:
Line 907: params['has_snapshots'] = ret > 0
--
To view, visit https://gerrit.ovirt.org/56574
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Golembiovsky
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik
Gerrit-Reviewer: Milan Zamazal
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Milan Zamazal has posted comments on this change.
Change subject: v2v: Detect VM with snapshots
..
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/56574/1/lib/vdsm/v2v.py
File lib/vdsm/v2v.py:
Line 899:
Line 900:
Line 901: def _add_has_snapshots(vm, params):
Line 902: try:
Line 903: ret = vm.hasCurrentSnapshot()
Does this method exist? I can see it only in tests.
Line 904: except libvirt.libvirtError:
Line 905: logging.exception("Error checking for existing snapshots")
Line 906: else:
Line 907: params['has_snapshots'] = ret > 0
--
To view, visit https://gerrit.ovirt.org/56574
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Golembiovsky
Gerrit-Reviewer: Francesco Romani
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik
Gerrit-Reviewer: Milan Zamazal
Gerrit-Reviewer: Vinzenz Feenstra
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Francesco Romani has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 1: Code-Review+1 (2 comments) codewise seems fine, a couple of questions about the tests? Partial ACK. https://gerrit.ovirt.org/#/c/56574/1/tests/v2vTests.py File tests/v2vTests.py: PS1, Line 383: if methodname == 'lookupByName': : # Only active domains : self.assertEqual(len(vms), 2) : else: # if methodname == 'lookupByID' : # Only inactive domains : self.assertEqual(len(vms), 3) why is this needed? PS1, Line 526: self.assertEqual(len(vms), : len([x for x in VM_SPECS if not x.active])) why is this needed? -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
gerrit-hooks has posted comments on this change. Change subject: v2v: Detect VM with snapshots .. Patch Set 1: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56574 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Tomas Golembiovsky Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: v2v: Detect VM with snapshots
Tomas Golembiovsky has uploaded a new change for review.
Change subject: v2v: Detect VM with snapshots
..
v2v: Detect VM with snapshots
virt-v2v cannot properly handle conversion ov VMware machine with
snapshots. Engine needs a way to know which machines have snapshots and
which do not to filter them out and/or notify the user.
Change-Id: I9aa4de2faff92625cd0de8e3ae2a10a2d58aa823
Signed-off-by: Tomáš Golembiovský
---
M lib/api/vdsmapi-schema.json
M lib/vdsm/v2v.py
M tests/v2vTests.py
3 files changed, 36 insertions(+), 9 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/74/56574/1
diff --git a/lib/api/vdsmapi-schema.json b/lib/api/vdsmapi-schema.json
index 4153f86..6ca50e2 100644
--- a/lib/api/vdsmapi-schema.json
+++ b/lib/api/vdsmapi-schema.json
@@ -4024,13 +4024,17 @@
#
# @virtio_iso_path: #optional path for iso contains virtio guest drivers
#
+# @has_snapshots: Whether the VM has snapshots or not.
+# (new in version 4.18.0)
+#
# Since: 4.17.0
##
{'type': 'ExternalVmInfo',
'data': {'vmName': 'str', 'status': 'VmStatus', 'vmId': 'UUID', 'smp': 'uint',
'memSize': 'uint', '*disks': ['ExternalDiskInfo'],
'*networks': ['ExternalNetworkInfo'], '*format': 'VolumeFormat',
- '*allocation': 'VolumeAllocation', '*virtio_iso_path': 'str'}}
+ '*allocation': 'VolumeAllocation', '*virtio_iso_path': 'str',
+ '*has_snapshots': 'bool'}}
##
# @Host.getExternalVMs:
diff --git a/lib/vdsm/v2v.py b/lib/vdsm/v2v.py
index f96bda6..406b88c 100644
--- a/lib/vdsm/v2v.py
+++ b/lib/vdsm/v2v.py
@@ -802,6 +802,7 @@
except InvalidVMConfiguration as e:
logging.error("error adding general info: %s", e)
return
+_add_has_snapshots(vm, params)
_add_networks(root, params)
_add_disks(root, params)
for disk in params['disks']:
@@ -897,6 +898,15 @@
params['networks'].append(i)
+def _add_has_snapshots(vm, params):
+try:
+ret = vm.hasCurrentSnapshot()
+except libvirt.libvirtError:
+logging.exception("Error checking for existing snapshots")
+else:
+params['has_snapshots'] = ret > 0
+
+
def _read_ovf_from_ova(ova_path):
"""
virt-v2v support ova in tar, zip formats as well as
diff --git a/tests/v2vTests.py b/tests/v2vTests.py
index bad613f..d178c0d 100644
--- a/tests/v2vTests.py
+++ b/tests/v2vTests.py
@@ -41,13 +41,14 @@
import vmfakelib as fake
-VmSpec = namedtuple('VmSpec', ['name', 'uuid', 'id', 'active'])
+VmSpec = namedtuple('VmSpec', ['name', 'uuid', 'id', 'active', 'snapshots'])
VM_SPECS = (
-VmSpec("RHEL_0", str(uuid.uuid4()), id=0, active=True),
-VmSpec("RHEL_1", str(uuid.uuid4()), id=1, active=True),
-VmSpec("RHEL_2", str(uuid.uuid4()), id=2, active=False),
-VmSpec("RHEL_3", str(uuid.uuid4()), id=3, active=False)
+VmSpec("RHEL_0", str(uuid.uuid4()), id=0, active=True, snapshots=False),
+VmSpec("RHEL_1", str(uuid.uuid4()), id=1, active=True, snapshots=False),
+VmSpec("RHEL_2", str(uuid.uuid4()), id=2, active=False, snapshots=False),
+VmSpec("RHEL_3", str(uuid.uuid4()), id=3, active=False, snapshots=False),
+VmSpec("RHEL_4", str(uuid.uuid4()), id=4, active=False, snapshots=True),
)
FAKE_VIRT_V2V = CommandPath('fake-virt-v2v',
@@ -68,12 +69,14 @@
def __init__(self, name="RHEL",
vm_uuid="564d7cb4-8e3d-06ec-ce82-7b2b13c6a611",
id=0,
- active=False):
+ active=False,
+ snapshots=False):
self._name = name
self._uuid = vm_uuid
self._mac_address = _mac_from_uuid(vm_uuid)
self._id = id
self._active = active
+self._snapshots = snapshots
def name(self):
return self._name
@@ -131,6 +134,9 @@
name=self._name,
uuid=self._uuid,
mac=self._mac_address)
+
+def hasCurrentSnapshot(self):
+return self._snapshots
# FIXME: extend vmfakelib allowing to set predefined domain in Connection class
@@ -374,7 +380,12 @@
vms = v2v.get_external_vms('esx://mydomain', 'user',
ProtectedPassword('password')
)['vmList']
-self.assertEqual(len(vms), 2)
+if methodname == 'lookupByName':
+# Only active domains
+self.assertEqual(len(vms), 2)
+else: # if methodname == 'lookupByID'
+# Only inactive domains
+self.assertEqual(len(vms), 3)
def testOutputParser(self):
output = ''.join(['[ 0.0] Opening the source -i libvirt ://roo...\n',
@@ -437,6 +448,7 @@
self.assertEquals(vm['smp'], 1)
self.assertEquals(len(vm['disks']), 1)
self.assertEquals(len(vm['networks']), 1)
+self.assertEquals(vm['has_snapshots'], spec.s
