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

Reply via email to