Change in vdsm[master]: virt: storage: avoid empty serials to domain XML

nsoffer at redhat.com nsoffer at redhat.com
Tue Apr 12 07:58:08 UTC 2016


Nir Soffer has posted comments on this change.

Change subject: virt: storage: avoid empty serials to domain XML
......................................................................


Patch Set 1: Code-Review+1

(2 comments)

Looks good, but can little nicer.

https://gerrit.ovirt.org/#/c/56014/1/tests/vmStorageTests.py
File tests/vmStorageTests.py:

Line 183:             index='2',
Line 184:             path='/path/to/fedora.iso',
Line 185:             readonly='True',
Line 186:         )
Line 187:         del conf['serial']
This is little ugly, better to generate the serial only if given a serial argument instead of automatically. Bigger change but cleaner code.
Line 188:         xml = """
Line 189:             <disk device="cdrom" snapshot="no" type="file">
Line 190:                 <source file="/path/to/fedora.iso" startupPolicy="optional"/>
Line 191:                 <target bus="ide" dev="hdc"/>


https://gerrit.ovirt.org/#/c/56014/1/vdsm/virt/vmdevices/storage.py
File vdsm/virt/vmdevices/storage.py:

Line 451:             # floppies are used only internally for sysprep, so
Line 452:             # they are readonly unless explicitely stated otherwise
Line 453:             diskelem.appendChildWithArgs('readonly')
Line 454: 
Line 455:         if hasattr(self, 'serial') and self.serial and self.device != 'lun':
We should use:

    if getattr(self, 'serial', False) and self.device != 'lun':
Line 456:             diskelem.appendChildWithArgs('serial', text=self.serial)
Line 457: 
Line 458:         if hasattr(self, 'bootOrder'):
Line 459:             diskelem.appendChildWithArgs('boot', order=self.bootOrder)


-- 
To view, visit https://gerrit.ovirt.org/56014
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I175954321faef8fb33ee31b6b367bfad9ec87137
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list