12:27 a.m.
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(a)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
12:27 a.m.
New subject: [libvirt] [PATCH 2/4] tests: Teach qemuxml2argvtest about spapr-vio addresses
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(a)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
12:27 a.m.
New subject: [libvirt] [PATCH 3/4] tests: Add qemuxml2argv tests for PPC64 pseries machine
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(a)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
12:27 a.m.
New subject: [libvirt] [PATCH 4/4] conf: Remove do-nothing validation functions
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(a)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
3:49 a.m.
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
4723
days inactive
4723
days old
4 comments
1 participants
participants (1)
-
Michael Ellerman