Martin Polednik has posted comments on this change. Change subject: cdrom: API change: require interface & index ......................................................................
Patch Set 3: (4 comments) https://gerrit.ovirt.org/#/c/56805/3/lib/api/vdsm-api.yml File lib/api/vdsm-api.yml: Line 745: cdrom. Line 746: name: DriveSpecCdrom Line 747: type: object Line 748: properties: Line 749: - description: The full filesystem path to the image file > Is this always an image file, or we use the same api to connect to a real c Only file (not changing the behavior at the moment / not aim of these changes). Line 750: name: path Line 751: type: string Line 752: Line 753: - description: Bus that the cdrom resides on https://gerrit.ovirt.org/#/c/56805/3/lib/api/vdsmapi-schema.json File lib/api/vdsmapi-schema.json: Line 6497: ## Line 6498: {'type': 'DriveSpec', Line 6499: 'data': {}, Line 6500: 'union': ['DriveSpecVolume', 'DriveSpecGUID', 'DriveSpecPayload', Line 6501: 'DriveSpecPath', 'DriveSpecCdrom', 'str']} > Can you document here the 'str' type, and warn that it is deprecated? Worked on the alias (will be visible in prev. patch). Line 6502: Line 6503: ## Line 6504: # @VM.changeCD: Line 6505: # https://gerrit.ovirt.org/#/c/56805/3/tests/vmTests.py File tests/vmTests.py: Line 1598: # no specific meaning, actually any error != None is good Line 1599: fakevm._dom = fake.Domain( Line 1600: virtError=libvirt.VIR_ERR_GET_FAILED) Line 1601: Line 1602: res = fakevm.changeCD('') > This is confusing, if you are trying to test a libvirt failume, use here a True. Line 1603: Line 1604: expected_status = define.errCode['changeDisk']['status'] Line 1605: self.assertEqual(res['status'], expected_status) Line 1606: https://gerrit.ovirt.org/#/c/56805/3/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 3864: "Action: %s", self.name, Line 3865: actionToString(action)) Line 3866: Line 3867: def changeCD(self, cdromspec): Line 3868: if isinstance(cdromspec, str): > Nice! Done Line 3869: # < 4.0 - known cdrom interface/index Line 3870: drivespec = cdromspec Line 3871: if cpuarch.is_ppc(self.arch): Line 3872: blockdev = 'sda' -- To view, visit https://gerrit.ovirt.org/56805 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I178c1a02bbad962f9dc9b67bed7691cf170ee896 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches