Dan Kenigsberg has posted comments on this change. Change subject: v2v: add mount windows iso drivers ......................................................................
Patch Set 1: Code-Review-1 (4 comments) https://gerrit.ovirt.org/#/c/42358/1//COMMIT_MSG Commit Message: Line 6: Line 7: v2v: add mount windows iso drivers Line 8: Line 9: virt-v2v expect proprietary Windows drivers to be in: Line 10: /usr/share/virtio-win or the path sould be be set in VIRTIO_WIN_DIR environment. where can this ISO file be obtained? This patch mounts it under /var/run, but it does not set VIRTIO_WIN_DIR env. Line 11: Line 12: Change-Id: I675555d29bc354fff3b393e83f6b9594fe5bdd75 https://gerrit.ovirt.org/#/c/42358/1/vdsm/v2v.py File vdsm/v2v.py: Line 49: _jobs = {} Line 50: Line 51: _V2V_DIR = os.path.join(P_VDSM_RUN, 'v2v') Line 52: _VIRT_V2V = CommandPath('virt-v2v', '/usr/bin/virt-v2v') Line 53: _WIN_DRIVER_DIR = _V2V_DIR = os.path.join(_V2V_DIR, 'virtio-win') are you intentionally re-writing the constant _V2V_DIR? this is very odd. Line 54: Line 55: ImportProgress = namedtuple('ImportProgress', Line 56: ['current_disk', 'disk_count', 'description']) Line 57: DiskProgress = namedtuple('DiskProgress', ['progress']) Line 178: return errCode[e.err_name] Line 179: return {'status': doneCode, 'ovf': ovf} Line 180: Line 181: Line 182: def mount_windows_drivers(iso_path, umount=False): I dislike functions that do two different functions (like mount and unmount). If we have to, we can expose two verbs. Line 183: if not os.path.exists(iso_path): Line 184: raise InvalidInputError('Iso file not exists %s' % iso_path) Line 185: Line 186: if not os.path.exists(_WIN_DRIVER_DIR): Line 183: if not os.path.exists(iso_path): Line 184: raise InvalidInputError('Iso file not exists %s' % iso_path) Line 185: Line 186: if not os.path.exists(_WIN_DRIVER_DIR): Line 187: cmd = ['/usr/bin/mkdir', _WIN_DRIVER_DIR] we have os.makedirs() for this. Line 188: rc, out, err = execCmd(cmd) Line 189: if rc != 0: Line 190: raise V2VError('Error mounting iso: %r' % err) Line 191: -- To view, visit https://gerrit.ovirt.org/42358 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I675555d29bc354fff3b393e83f6b9594fe5bdd75 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
