[libvirt] [PATCH 1/4] Make drive unit attribute optional in the XML schema
The "unit" attribute of a drive address is optional in the code, so should also be in the XML schema. Signed-off-by: Michael Ellerman <michael@ellerman.id.au> --- docs/schemas/domaincommon.rng | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 553a6f0..f35eb0b 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2241,9 +2241,11 @@ <ref name="driveBus"/> </attribute> </optional> - <attribute name="unit"> - <ref name="driveUnit"/> - </attribute> + <optional> + <attribute name="unit"> + <ref name="driveUnit"/> + </attribute> + </optional> </define> <define name="virtioserialaddress"> <attribute name="controller"> -- 1.7.7.3
We can't call qemuCapsExtractVersionInfo() from test code, because it expects to be able to call the emulator, and for testing we have fake emulators that can't be executed. For that reason qemuxml2argvtest.c doesn't call qemuDomainAssignPCIAddresses(), instead it open codes its own version. That means we can't call qemuDomainAssignAddresses() from the test code, instead we need to manually call qemuDomainAssignSpaprVioAddresses(). Also add logic to cope with qemuDomainAssignSpaprVioAddresses() failing, so that we can write a test that checks for a known failure in there. Signed-off-by: Michael Ellerman <michael@ellerman.id.au> --- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_command.h | 1 + tests/qemuxml2argvtest.c | 18 +++++++++++++----- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9d3bc23..b241147 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -735,7 +735,7 @@ qemuAssignSpaprVIOAddress(virDomainDefPtr def, virDomainDeviceInfoPtr info, return 0; } -static int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def) +int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def) { int i, rc; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index de61cf3..2f8b5ba 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -175,6 +175,7 @@ virDomainDefPtr qemuParseCommandLinePid(virCapsPtr caps, bool *monJSON); int qemuDomainAssignAddresses(virDomainDefPtr def); +int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def); int qemuDomainAssignPCIAddresses(virDomainDefPtr def); qemuDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e1221eb..401faea 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -153,6 +153,13 @@ static int testCompareXMLToArgvFiles(const char *xml, if (qemuCapsGet(extraFlags, QEMU_CAPS_DEVICE)) { qemuDomainPCIAddressSetPtr pciaddrs; + + if (qemuDomainAssignSpaprVIOAddresses(vmdef)) { + if (expectError) + goto ok; + goto fail; + } + if (!(pciaddrs = qemuDomainPCIAddressSetCreate(vmdef))) goto fail; @@ -190,11 +197,6 @@ static int testCompareXMLToArgvFiles(const char *xml, goto fail; } - if (expectError) { - /* need to suppress the errors */ - virResetLastError(); - } - if (!(actualargv = virCommandToString(cmd))) goto fail; @@ -212,6 +214,12 @@ static int testCompareXMLToArgvFiles(const char *xml, goto fail; } + ok: + if (expectError) { + /* need to suppress the errors */ + virResetLastError(); + } + ret = 0; fail: -- 1.7.7.3
Add four tests of the XML -> argv handling for the PPC64 pseries machine. The first is just a basic test of a bare bones machine. The three others test various aspects of the spapr-vio address handling. It seems that currently we can't include network devices, doing so leads to a segfault because the network driverState is not initialised. Working around that leads us to the problem that the 'default' network doesn't exist. So for now just leave network devices out. Signed-off-by: Michael Ellerman <michael@ellerman.id.au> --- .../qemuxml2argv-pseries-basic.args | 1 + .../qemuxml2argv-pseries-basic.xml | 17 ++++++++ .../qemuxml2argv-pseries-vio-address-clash.xml | 42 ++++++++++++++++++++ .../qemuxml2argv-pseries-vio-user-assigned.args | 1 + .../qemuxml2argv-pseries-vio-user-assigned.xml | 42 ++++++++++++++++++++ .../qemuxml2argvdata/qemuxml2argv-pseries-vio.args | 1 + .../qemuxml2argvdata/qemuxml2argv-pseries-vio.xml | 42 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 9 ++++ 8 files changed, 155 insertions(+), 0 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-basic.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-basic.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-address-clash.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-address-clash.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-user-assigned.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-user-assigned.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-vio.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-vio.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-basic.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-basic.args new file mode 100644 index 0000000..f9aec92 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-basic.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu-system-ppc64 -S -M pseries -m 512 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -chardev pty,id=charserial0 -device spapr-vty,chardev=charserial0,reg=0x30000000 -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-basic.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-basic.xml new file mode 100644 index 0000000..1b164f9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-basic.xml @@ -0,0 +1,17 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory>524288</memory> + <vcpu>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <clock offset='utc'/> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <console type='pty'> + <address type="spapr-vio"/> + </console> + <memballoon model="none"/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-address-clash.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-address-clash.args new file mode 100644 index 0000000..e69de29 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-address-clash.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-address-clash.xml new file mode 100644 index 0000000..4504f06 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-address-clash.xml @@ -0,0 +1,42 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>2754dd7b-ac8a-4850-aec0-1f3fcd43235b</uuid> + <memory>524288</memory> + <vcpu>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <clock offset='utc'/> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <console type='pty'> + <address type="spapr-vio"/> + </console> + + <!-- Two serials, first is the console --> + <serial type="pty"> + <address type="spapr-vio"/> + </serial> + <serial type="pty"> + <address type="spapr-vio" reg="0x4000"/> + </serial> + + <!-- One disk --> + <disk type="file" device="disk"> + <driver name="qemu" type="raw"/> + <source file="/tmp/scsidisk.img"/> + <target dev="sda" bus="scsi"/> + <address type="drive" controller="1"/> + </disk> + + <!-- Two SCSI controllers --> + <controller type="scsi" index="1"> + <address type="spapr-vio"/> + </controller> + <controller type="scsi" index="0"> + <address type="spapr-vio" reg="0x4000"/> + </controller> + + <memballoon model="none"/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-user-assigned.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-user-assigned.args new file mode 100644 index 0000000..e939e1b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-user-assigned.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu-system-ppc64 -S -M pseries -m 512 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device spapr-vscsi,id=scsi0,reg=0x2000 -device spapr-vscsi,id=scsi1,reg=0x30000000 -drive file=/tmp/scsidisk.img,if=none,id=drive-scsi1-0-0 -device scsi-disk,bus=scsi1.0,scsi-id=0,drive=drive-scsi1-0-0,id=scsi1-0-0 -chardev pty,id=charserial0 -device spapr-vty,chardev=charserial0,reg=0x20000000 -chardev pty,id=charserial1 -device spapr-vty,chardev=charserial1,reg=0x30001000 -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-user-assigned.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-user-assigned.xml new file mode 100644 index 0000000..cf65478 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio-user-assigned.xml @@ -0,0 +1,42 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>2754dd7b-ac8a-4850-aec0-1f3fcd43235b</uuid> + <memory>524288</memory> + <vcpu>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <clock offset='utc'/> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <console type='pty'> + <address type="spapr-vio"/> + </console> + + <!-- Two serials, first is the console --> + <serial type="pty"> + <address type="spapr-vio" reg="0x20000000"/> + </serial> + <serial type="pty"> + <address type="spapr-vio"/> + </serial> + + <!-- One disk --> + <disk type="file" device="disk"> + <driver name="qemu" type="raw"/> + <source file="/tmp/scsidisk.img"/> + <target dev="sda" bus="scsi"/> + <address type="drive" controller="1"/> + </disk> + + <!-- Two SCSI controllers --> + <controller type="scsi" index="1"> + <address type="spapr-vio" reg="0x30000000"/> + </controller> + <controller type="scsi" index="0"> + <address type="spapr-vio"/> + </controller> + + <memballoon model="none"/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio.args new file mode 100644 index 0000000..5fe0c88 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu-system-ppc64 -S -M pseries -m 512 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device spapr-vscsi,id=scsi0,reg=0x2000 -device spapr-vscsi,id=scsi1,reg=0x3000 -drive file=/tmp/scsidisk.img,if=none,id=drive-scsi1-0-0 -device scsi-disk,bus=scsi1.0,scsi-id=0,drive=drive-scsi1-0-0,id=scsi1-0-0 -chardev pty,id=charserial0 -device spapr-vty,chardev=charserial0,reg=0x30000000 -chardev pty,id=charserial1 -device spapr-vty,chardev=charserial1,reg=0x30001000 -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio.xml new file mode 100644 index 0000000..68f4216 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-vio.xml @@ -0,0 +1,42 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>2754dd7b-ac8a-4850-aec0-1f3fcd43235b</uuid> + <memory>524288</memory> + <vcpu>1</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <clock offset='utc'/> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <console type='pty'> + <address type="spapr-vio"/> + </console> + + <!-- Two serials, first is the console --> + <serial type="pty"> + <address type="spapr-vio"/> + </serial> + <serial type="pty"> + <address type="spapr-vio"/> + </serial> + + <!-- One disk --> + <disk type="file" device="disk"> + <driver name="qemu" type="raw"/> + <source file="/tmp/scsidisk.img"/> + <target dev="sda" bus="scsi"/> + <address type="drive" controller="1"/> + </disk> + + <!-- Two SCSI controllers --> + <controller type="scsi" index="1"> + <address type="spapr-vio"/> + </controller> + <controller type="scsi" index="0"> + <address type="spapr-vio"/> + </controller> + + <memballoon model="none"/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 401faea..593fd61 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -668,6 +668,15 @@ mymain(void) QEMU_CAPS_CHARDEV, QEMU_CAPS_MONITOR_JSON, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_NO_SHUTDOWN); + DO_TEST("pseries-basic", false, + QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + DO_TEST("pseries-vio", false, QEMU_CAPS_DRIVE, + QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + DO_TEST("pseries-vio-user-assigned", false, QEMU_CAPS_DRIVE, + QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + DO_TEST("pseries-vio-address-clash", true, QEMU_CAPS_DRIVE, + QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); + free(driver.stateDir); virCapabilitiesFree(driver.caps); free(map); -- 1.7.7.3
There are three address validation routines that do nothing: virDomainDeviceDriveAddressIsValid() virDomainDeviceUSBAddressIsValid() virDomainDeviceVirtioSerialAddressIsValid() Remove them, and replace their call sites with "1" which is what they currently return. In some cases this means we can remove an entire if block. Signed-off-by: Michael Ellerman <michael@ellerman.id.au> --- src/conf/domain_conf.c | 36 ++---------------------------------- src/conf/domain_conf.h | 3 --- 2 files changed, 2 insertions(+), 37 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2897b4a..c01d07a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1750,10 +1750,10 @@ int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, return virDomainDevicePCIAddressIsValid(&info->addr.pci); case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE: - return virDomainDeviceDriveAddressIsValid(&info->addr.drive); + return 1; case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB: - return virDomainDeviceUSBAddressIsValid(&info->addr.usb); + return 1; } return 0; @@ -1769,24 +1769,6 @@ int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr) } -int virDomainDeviceDriveAddressIsValid(virDomainDeviceDriveAddressPtr addr ATTRIBUTE_UNUSED) -{ - /*return addr->controller || addr->bus || addr->unit;*/ - return 1; /* 0 is valid for all fields, so any successfully parsed addr is valid */ -} - -int virDomainDeviceUSBAddressIsValid(virDomainDeviceUSBAddressPtr addr ATTRIBUTE_UNUSED) -{ - return 1; /* FIXME.. any successfully parsed addr is valid */ -} - -int virDomainDeviceVirtioSerialAddressIsValid( - virDomainDeviceVirtioSerialAddressPtr addr ATTRIBUTE_UNUSED) -{ - return 1; /* 0 is valid for all fields, so any successfully parsed addr is valid */ -} - - static int virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info, unsigned int flags) { @@ -1797,7 +1779,6 @@ virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info, unsigned int flags) return 0; } - void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info) { VIR_FREE(info->alias); @@ -2082,12 +2063,6 @@ virDomainDeviceDriveAddressParseXML(xmlNodePtr node, goto cleanup; } - if (!virDomainDeviceDriveAddressIsValid(addr)) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Insufficient specification for drive address")); - goto cleanup; - } - ret = 0; cleanup: @@ -2134,13 +2109,6 @@ virDomainDeviceVirtioSerialAddressParseXML( goto cleanup; } - if (!virDomainDeviceVirtioSerialAddressIsValid(addr)) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Insufficient specification for " - "virtio serial address")); - goto cleanup; - } - ret = 0; cleanup: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1f6e442..4b65792 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1701,9 +1701,6 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def); int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, int type); int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr); -int virDomainDeviceDriveAddressIsValid(virDomainDeviceDriveAddressPtr addr); -int virDomainDeviceVirtioSerialAddressIsValid(virDomainDeviceVirtioSerialAddressPtr addr); -int virDomainDeviceUSBAddressIsValid(virDomainDeviceUSBAddressPtr addr); void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info); void virDomainDefClearPCIAddresses(virDomainDefPtr def); void virDomainDefClearDeviceAliases(virDomainDefPtr def); -- 1.7.7.3
On Wed, 2011-12-21 at 17:27 +1100, Michael Ellerman wrote:
The "unit" attribute of a drive address is optional in the code, so should also be in the XML schema.
Sorry for all the confusion with testing the previous version of this series. With the first three patches here I am seeing only one failure for make check, which AFAICS is not my fault - ie. it fails with and without my patches. The domainschematest is passing as are the xml2argv tests. The fourth patch is just an optional cleanup that jumped out at me. cheers
participants (1)
-
Michael Ellerman