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 d051305..4ceb01c 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 d87654c..67af27a 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 67af27a..87c05f3 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -684,6 +684,15 @@ mymain(void) DO_TEST("seclabel-static", false, QEMU_CAPS_NAME); DO_TEST("seclabel-static-relabel", false, QEMU_CAPS_NAME); + 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 5e36270..4d04f35 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1759,10 +1759,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; @@ -1778,24 +1778,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) { @@ -1806,7 +1788,6 @@ virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info, unsigned int flags) return 0; } - void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info) { VIR_FREE(info->alias); @@ -2091,12 +2072,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: @@ -2143,13 +2118,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 3d5d4f8..0e14a26 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1702,9 +1702,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 01/12/2012 08:39 PM, Michael Ellerman wrote:

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 e93ae77..b9da906 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2307,9 +2307,11 @@ <ref name="driveBus"/> </attribute> </optional> - <attribute name="unit"> - <ref name="driveUnit"/> - </attribute> + <optional> + <attribute name="unit"> + <ref name="driveUnit"/> + </attribute> + </optional>

ACK and pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org