[libvirt] PATCH 0/0: Support setting a disk drive serial (number) v2
This is a second version of the patch posted here http://www.redhat.com/archives/libvir-list/2009-August/msg00341.html In this version, I make the QEMU driver refuse to run with any serial that doesn't contain 0-9, a-Z, -, _, since there's no way to escape special chars in QEMU command line args. Hopefully we can lift this restriction in a future version of QEMU I also add a check that serial=s is actually supported by QEMU
* docs/schemas/domain.rng: Add <serial> element to disks * src/domain_conf.h, src/domain_conf.c: XML parsing and formatting for disk serial numbers * src/qemu_conf.c: Set serial number when launching guests * tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args, tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml: Add serial number to XML test --- docs/schemas/domain.rng | 10 ++++++++ src/domain_conf.c | 11 ++++++++ src/domain_conf.h | 1 + src/qemu_conf.c | 25 ++++++++++++++++++++ src/qemu_conf.h | 1 + tests/qemuhelptest.c | 3 ++ .../qemuxml2argv-disk-drive-shared.args | 2 +- .../qemuxml2argv-disk-drive-shared.xml | 1 + tests/qemuxml2argvtest.c | 2 +- 9 files changed, 54 insertions(+), 2 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index f857301..16f35e4 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -336,6 +336,11 @@ <empty/> </element> </optional> + <optional> + <element name="serial"> + <ref name="diskSerial"/> + </element> + </optional> </define> <!-- A disk description can be either of type file or block @@ -1135,6 +1140,11 @@ <param name="pattern">[A-Za-z0-9_\.\+\-&:/]+</param> </data> </define> + <define name="diskSerial"> + <data type="string"> + <param name="pattern">[A-Za-z0-9_\.\+\-]+</param> + </data> + </define> <define name="genericName"> <data type="string"> <param name="pattern">[a-zA-Z0-9_\+\-]+</param> diff --git a/src/domain_conf.c b/src/domain_conf.c index 1d2cc7c..654f753 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -284,6 +284,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) if (!def) return; + VIR_FREE(def->serial); VIR_FREE(def->src); VIR_FREE(def->dst); VIR_FREE(def->driverName); @@ -661,6 +662,7 @@ virDomainDiskDefParseXML(virConnectPtr conn, char *bus = NULL; char *cachetag = NULL; char *devaddr = NULL; + char *serial = NULL; if (VIR_ALLOC(def) < 0) { virReportOOMError(conn); @@ -718,6 +720,9 @@ virDomainDiskDefParseXML(virConnectPtr conn, } else if ((flags & VIR_DOMAIN_XML_INTERNAL_STATUS) && xmlStrEqual(cur->name, BAD_CAST "state")) { devaddr = virXMLPropString(cur, "devaddr"); + } else if ((serial == NULL) && + (xmlStrEqual(cur->name, BAD_CAST "serial"))) { + serial = (char *)xmlNodeGetContent(cur); } } cur = cur->next; @@ -836,6 +841,8 @@ virDomainDiskDefParseXML(virConnectPtr conn, driverName = NULL; def->driverType = driverType; driverType = NULL; + def->serial = serial; + serial = NULL; cleanup: VIR_FREE(bus); @@ -847,6 +854,7 @@ cleanup: VIR_FREE(driverName); VIR_FREE(cachetag); VIR_FREE(devaddr); + VIR_FREE(serial); return def; @@ -3519,6 +3527,9 @@ virDomainDiskDefFormat(virConnectPtr conn, virBufferAddLit(buf, " <readonly/>\n"); if (def->shared) virBufferAddLit(buf, " <shareable/>\n"); + if (def->serial) + virBufferEscapeString(buf, " <serial>%s</serial>\n", + def->serial); if (flags & VIR_DOMAIN_XML_INTERNAL_STATUS) { virBufferAddLit(buf, " <state"); diff --git a/src/domain_conf.h b/src/domain_conf.h index 44302be..7bad5e7 100644 --- a/src/domain_conf.h +++ b/src/domain_conf.h @@ -109,6 +109,7 @@ struct _virDomainDiskDef { char *dst; char *driverName; char *driverType; + char *serial; int cachemode; unsigned int readonly : 1; unsigned int shared : 1; diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 22f5edd..381f281 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -782,6 +782,8 @@ static unsigned int qemudComputeCmdFlags(const char *help, flags |= QEMUD_CMD_FLAG_VGA; if (strstr(help, "boot=on")) flags |= QEMUD_CMD_FLAG_DRIVE_BOOT; + if (strstr(help, "serial=s")) + flags |= QEMUD_CMD_FLAG_DRIVE_SERIAL; if (strstr(help, "-pcidevice")) flags |= QEMUD_CMD_FLAG_PCIDEVICE; @@ -1375,6 +1377,23 @@ static int qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev, return 0; } +#define QEMU_SERIAL_PARAM_ACCEPTED_CHARS \ + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_" + +static int +qemuSafeSerialParamValue(virConnectPtr conn, + const char *value) +{ + if (strspn(value, QEMU_SERIAL_PARAM_ACCEPTED_CHARS) != strlen (value)) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("driver serial '%s' contains unsafe characters"), + value); + return -1; + } + + return 0; +} + /* * Constructs a argv suitable for launching qemu with config defined * for a given virtual machine. @@ -1754,6 +1773,12 @@ int qemudBuildCommandLine(virConnectPtr conn, if (disk->driverType && qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_FORMAT) virBufferVSprintf(&opt, ",format=%s", disk->driverType); + if (disk->serial && + (qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_SERIAL)) { + if (qemuSafeSerialParamValue(conn, disk->serial) < 0) + goto error; + virBufferVSprintf(&opt, ",serial=%s", disk->serial); + } if (disk->cachemode) { const char *mode = diff --git a/src/qemu_conf.h b/src/qemu_conf.h index a126dac..dcfacb9 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -67,6 +67,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_HOST_NET_ADD = QEMUD_CMD_FLAG_0_10, /* host_net_add monitor command */ QEMUD_CMD_FLAG_PCIDEVICE = (1 << 17), /* PCI device assignment only supported by qemu-kvm */ + QEMUD_CMD_FLAG_DRIVE_SERIAL = (1 << 18), /* -driver serial= available */ }; /* Main driver state */ diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index ad2045f..e21f4c4 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -118,6 +118,7 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC | QEMUD_CMD_FLAG_DRIVE_CACHE_V2 | QEMUD_CMD_FLAG_DRIVE_FORMAT | + QEMUD_CMD_FLAG_DRIVE_SERIAL | QEMUD_CMD_FLAG_VGA | QEMUD_CMD_FLAG_0_10, 10005, 0, 0); @@ -134,6 +135,7 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_DRIVE_CACHE_V2 | QEMUD_CMD_FLAG_KVM | QEMUD_CMD_FLAG_DRIVE_FORMAT | + QEMUD_CMD_FLAG_DRIVE_SERIAL | QEMUD_CMD_FLAG_VGA | QEMUD_CMD_FLAG_0_10 | QEMUD_CMD_FLAG_PCIDEVICE, @@ -151,6 +153,7 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_DRIVE_CACHE_V2 | QEMUD_CMD_FLAG_KVM | QEMUD_CMD_FLAG_DRIVE_FORMAT | + QEMUD_CMD_FLAG_DRIVE_SERIAL | QEMUD_CMD_FLAG_VGA | QEMUD_CMD_FLAG_0_10 | QEMUD_CMD_FLAG_PCIDEVICE, diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args index 611cd33..07cbe0e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,index=0,format=qcow2,cache=off -drive file=/dev/HostVG/QEMUGuest2,if=ide,media=cdrom,index=2,format=raw -net none -serial none -parallel none -usb +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,index=0,format=qcow2,serial=XYZXYZXYZYXXYZYZYXYZY,cache=off -drive file=/dev/HostVG/QEMUGuest2,if=ide,media=cdrom,index=2,format=raw -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml index b386315..c4e4734 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml @@ -19,6 +19,7 @@ <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='hda' bus='ide'/> <shareable/> + <serial>XYZXYZXYZYXXYZYZYXYZY</serial> </disk> <disk type='block' device='cdrom'> <driver name='qemu' type='raw'/> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 6f25e7d..f27a056 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -204,7 +204,7 @@ mymain(int argc, char **argv) DO_TEST("disk-drive-fmt-qcow", QEMUD_CMD_FLAG_DRIVE | QEMUD_CMD_FLAG_DRIVE_BOOT | QEMUD_CMD_FLAG_DRIVE_FORMAT); DO_TEST("disk-drive-shared", QEMUD_CMD_FLAG_DRIVE | - QEMUD_CMD_FLAG_DRIVE_FORMAT); + QEMUD_CMD_FLAG_DRIVE_FORMAT | QEMUD_CMD_FLAG_DRIVE_SERIAL); DO_TEST("disk-drive-cache-v1-wt", QEMUD_CMD_FLAG_DRIVE | QEMUD_CMD_FLAG_DRIVE_FORMAT); DO_TEST("disk-drive-cache-v1-wb", QEMUD_CMD_FLAG_DRIVE | -- 1.6.2.5
On Tue, Aug 25, 2009 at 01:58:28PM +0100, Daniel P. Berrange wrote:
* docs/schemas/domain.rng: Add <serial> element to disks * src/domain_conf.h, src/domain_conf.c: XML parsing and formatting for disk serial numbers * src/qemu_conf.c: Set serial number when launching guests * tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args, tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml: Add serial number to XML test [...] + if (strspn(value, QEMU_SERIAL_PARAM_ACCEPTED_CHARS) != strlen (value)) {
heh I didn't know strspn() ! [...]
+ if (disk->serial && + (qemuCmdFlags & QEMUD_CMD_FLAG_DRIVE_SERIAL)) { + if (qemuSafeSerialParamValue(conn, disk->serial) < 0) + goto error; + virBufferVSprintf(&opt, ",serial=%s", disk->serial); + }
if (disk->cachemode) { const char *mode = diff --git a/src/qemu_conf.h b/src/qemu_conf.h index a126dac..dcfacb9 100644 --- a/src/qemu_conf.h +++ b/src/qemu_conf.h @@ -67,6 +67,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_HOST_NET_ADD = QEMUD_CMD_FLAG_0_10, /* host_net_add monitor command */
QEMUD_CMD_FLAG_PCIDEVICE = (1 << 17), /* PCI device assignment only supported by qemu-kvm */ + QEMUD_CMD_FLAG_DRIVE_SERIAL = (1 << 18), /* -driver serial= available */
Hum, at some point we will have to make qemuCmdFlags a 64bit not urgent though :-) ACK, looks fine ! let's commit this Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 08/25/09 14:58, Daniel P. Berrange wrote:
This is a second version of the patch posted here
http://www.redhat.com/archives/libvir-list/2009-August/msg00341.html
In this version, I make the QEMU driver refuse to run with any serial that doesn't contain 0-9, a-Z, -, _, since there's no way to escape special chars in QEMU command line args. Hopefully we can lift this restriction in a future version of QEMU
latest git (post 0.11) can handle it already. -drive id=foo,<usualstuffhere> -set 'drive.foo.serial=&$%,73:8bf/' cheers, Gerd
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Gerd Hoffmann