[libvirt] [PATCH V3 0/2] Add support for QEMU file descriptor sets

The following patch series adds initial support for QEMU file descriptor sets implementing support for creating the proper command line. Some devices are using the sets now. Regards, Stefan

Create field in virDomainDeviceInfo structure to hold file descriptor set the device is associated with. Have the number written into the Device Info XML and parsed from the XML. Remember the next-toy use file descriptor set in the QEMU private domain structure. Upon libvirt restart determine the maximum file descriptor set used in the Device Info XML and remember the next-to-use file descriptor set in the QEMU private domain structure. Upon termination of a domain, reset the next-to-use file descriptor set to its initial value '1'. Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/conf/capabilities.h | 5 ++++- src/conf/domain_conf.c | 20 +++++++++++++++++--- src/conf/domain_conf.h | 1 + src/lxc/lxc_domain.c | 3 ++- src/qemu/qemu_domain.c | 23 ++++++++++++++++++++++- src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_process.c | 2 ++ 7 files changed, 50 insertions(+), 6 deletions(-) Index: libvirt/src/qemu/qemu_domain.c =================================================================== --- libvirt.orig/src/qemu/qemu_domain.c +++ libvirt/src/qemu/qemu_domain.c @@ -219,6 +219,7 @@ static void *qemuDomainObjPrivateAlloc(v goto error; priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; + priv->nextfdset = 1; return priv; @@ -329,7 +330,25 @@ static int qemuDomainObjPrivateXMLFormat return 0; } -static int qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void *data) +static int qemuDomainRebuildFDSet(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceDefPtr dev ATTRIBUTE_UNUSED, + virDomainDeviceInfoPtr info, + void *opaque) +{ + int *nextfdset = opaque; + + if (info->fdset > 0 && *nextfdset <= info->fdset) { + *nextfdset = info->fdset + 1; + VIR_DEBUG("Found fdset %u for domain %s. Setting nextfdset = %u", + info->fdset, def->name, *nextfdset); + } + + + return 0; +} + +static int qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void *data, + virDomainDefPtr def) { qemuDomainObjPrivatePtr priv = data; char *monitorpath; @@ -471,6 +490,8 @@ static int qemuDomainObjPrivateXMLParse( priv->fakeReboot = virXPathBoolean("boolean(./fakereboot)", ctxt) == 1; + virDomainDeviceInfoIterate(def, qemuDomainRebuildFDSet, &priv->nextfdset); + return 0; error: Index: libvirt/src/qemu/qemu_domain.h =================================================================== --- libvirt.orig/src/qemu/qemu_domain.h +++ libvirt/src/qemu/qemu_domain.h @@ -160,6 +160,8 @@ struct _qemuDomainObjPrivate { qemuDomainCleanupCallback *cleanupCallbacks; size_t ncleanupCallbacks; size_t ncleanupCallbacks_max; + + unsigned int nextfdset; }; struct qemuDomainWatchdogEvent Index: libvirt/src/conf/domain_conf.h =================================================================== --- libvirt.orig/src/conf/domain_conf.h +++ libvirt/src/conf/domain_conf.h @@ -262,6 +262,7 @@ struct _virDomainDeviceInfo { * to consider the new fields */ char *alias; + unsigned int fdset; /* > 0; "-add-fd set=%u,fd=123", fdsetnum */ int type; union { virDevicePCIAddress pci; Index: libvirt/src/conf/domain_conf.c =================================================================== --- libvirt.orig/src/conf/domain_conf.c +++ libvirt/src/conf/domain_conf.c @@ -2219,7 +2219,10 @@ virDomainDeviceInfoFormat(virBufferPtr b if (info->alias && !(flags & VIR_DOMAIN_XML_INACTIVE)) { - virBufferAsprintf(buf, " <alias name='%s'/>\n", info->alias); + virBufferAsprintf(buf, " <alias name='%s'", info->alias); + if (info->fdset > 0) + virBufferAsprintf(buf, " fdset='%u'", info->fdset); + virBufferAddLit(buf, "/>\n"); } if (info->mastertype == VIR_DOMAIN_CONTROLLER_MASTER_USB) { @@ -2597,6 +2600,7 @@ virDomainDeviceInfoParseXML(xmlNodePtr n xmlNodePtr boot = NULL; xmlNodePtr rom = NULL; char *type = NULL; + char *fdset = NULL; int ret = -1; virDomainDeviceInfoClear(info); @@ -2627,9 +2631,18 @@ virDomainDeviceInfoParseXML(xmlNodePtr n cur = cur->next; } - if (alias) + if (alias) { info->alias = virXMLPropString(alias, "name"); + fdset = virXMLPropString(alias, "fdset"); + if (fdset && virStrToLong_ui(fdset, NULL, 10, &info->fdset) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("incorrect fdset '%s', expecting positive integer"), + fdset); + goto cleanup; + } + } + if (master) { info->mastertype = VIR_DOMAIN_CONTROLLER_MASTER_USB; if (virDomainDeviceUSBMasterParseXML(master, &info->master.usb) < 0) @@ -2716,6 +2729,7 @@ cleanup: if (ret == -1) VIR_FREE(info->alias); VIR_FREE(type); + VIR_FREE(fdset); return ret; } @@ -10563,7 +10577,7 @@ static virDomainObjPtr virDomainObjParse VIR_FREE(nodes); if (caps->privateDataXMLParse && - ((caps->privateDataXMLParse)(ctxt, obj->privateData)) < 0) + ((caps->privateDataXMLParse)(ctxt, obj->privateData, obj->def)) < 0) goto error; return obj; Index: libvirt/src/conf/capabilities.h =================================================================== --- libvirt.orig/src/conf/capabilities.h +++ libvirt/src/conf/capabilities.h @@ -149,6 +149,9 @@ struct _virDomainXMLNamespace { virDomainDefNamespaceHref href; }; +typedef struct _virDomainDef virDomainDef; +typedef virDomainDef *virDomainDefPtr; + typedef struct _virCaps virCaps; typedef virCaps* virCapsPtr; struct _virCaps { @@ -164,7 +167,7 @@ struct _virCaps { void *(*privateDataAllocFunc)(void); void (*privateDataFreeFunc)(void *); int (*privateDataXMLFormat)(virBufferPtr, void *); - int (*privateDataXMLParse)(xmlXPathContextPtr, void *); + int (*privateDataXMLParse)(xmlXPathContextPtr, void *, virDomainDefPtr); bool hasWideScsiBus; const char *defaultInitPath; Index: libvirt/src/lxc/lxc_domain.c =================================================================== --- libvirt.orig/src/lxc/lxc_domain.c +++ libvirt/src/lxc/lxc_domain.c @@ -57,7 +57,8 @@ static int virLXCDomainObjPrivateXMLForm return 0; } -static int virLXCDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void *data) +static int virLXCDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void *data, + virDomainDefPtr def ATTRIBUTE_UNUSED) { virLXCDomainObjPrivatePtr priv = data; unsigned long long thepid; Index: libvirt/src/qemu/qemu_process.c =================================================================== --- libvirt.orig/src/qemu/qemu_process.c +++ libvirt/src/qemu/qemu_process.c @@ -4216,6 +4216,8 @@ void qemuProcessStop(virQEMUDriverPtr dr priv->monConfig = NULL; } + priv->nextfdset = 1; + /* shut it off for sure */ ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE| VIR_QEMU_PROCESS_KILL_NOCHECK));

On Mon, Feb 04, 2013 at 07:12:37PM -0500, Stefan Berger wrote:
Create field in virDomainDeviceInfo structure to hold file descriptor set the device is associated with. Have the number written into the Device Info XML and parsed from the XML.
Remember the next-toy use file descriptor set in the QEMU private domain structure. Upon libvirt restart determine the maximum file descriptor set used in the Device Info XML and remember the next-to-use file descriptor set in the QEMU private domain structure.
Upon termination of a domain, reset the next-to-use file descriptor set to its initial value '1'.
Stefan Berger <stefanb@linux.vnet.ibm.com>
--- src/conf/capabilities.h | 5 ++++- src/conf/domain_conf.c | 20 +++++++++++++++++--- src/conf/domain_conf.h | 1 + src/lxc/lxc_domain.c | 3 ++- src/qemu/qemu_domain.c | 23 ++++++++++++++++++++++- src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_process.c | 2 ++ 7 files changed, 50 insertions(+), 6 deletions(-)
I really don't like the fact that we're modifying the virDomainDef structs, which are intended to hold static config description, to also hold QEMU specific dynamic state. We did this in the past before we had the per-driver 'privateData' associated with virDomainObjPtr instances. I really want to see this stuff remain entirely in the QEMU codebase and not pollute the generic domain_conf code Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 02/07/2013 10:12 AM, Daniel P. Berrange wrote:
On Mon, Feb 04, 2013 at 07:12:37PM -0500, Stefan Berger wrote:
Create field in virDomainDeviceInfo structure to hold file descriptor set the device is associated with. Have the number written into the Device Info XML and parsed from the XML.
Remember the next-toy use file descriptor set in the QEMU private domain structure. Upon libvirt restart determine the maximum file descriptor set used in the Device Info XML and remember the next-to-use file descriptor set in the QEMU private domain structure.
Upon termination of a domain, reset the next-to-use file descriptor set to its initial value '1'.
Stefan Berger <stefanb@linux.vnet.ibm.com>
--- src/conf/capabilities.h | 5 ++++- src/conf/domain_conf.c | 20 +++++++++++++++++--- src/conf/domain_conf.h | 1 + src/lxc/lxc_domain.c | 3 ++- src/qemu/qemu_domain.c | 23 ++++++++++++++++++++++- src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_process.c | 2 ++ 7 files changed, 50 insertions(+), 6 deletions(-) I really don't like the fact that we're modifying the virDomainDef structs, which are intended to hold static config description, to also hold QEMU specific dynamic state. We did this in the past before we had the per-driver 'privateData' associated with virDomainObjPtr instances.
In these patches we are storing the fdset that's in use by a particular device in the virDomainDeviceInfo structure and are persisting that fdset number next to the device's alias into XML written into /var/run/libvirt/qemu to be able to restore it upon libvirt restart. Now we would either need a similar per-device structure in parallel to those virDomainDeviceInfo structures or connect a QEMU private data structure to that virDomainDeviceInfo structure for storing the dynamic data and to keep reference back to the device. Not sure what's the best path ... This is how the per-device XML in /var/run/libvirt/qemu/xyz.xml looks now: <serial type='pipe'> <source path='/tmp/testpipe'/> <target port='1'/> <alias name='serial1' fdset='3'/> </serial> Regards, Stefan

On Thu, Feb 07, 2013 at 12:56:45PM -0500, Stefan Berger wrote:
On 02/07/2013 10:12 AM, Daniel P. Berrange wrote:
On Mon, Feb 04, 2013 at 07:12:37PM -0500, Stefan Berger wrote:
Create field in virDomainDeviceInfo structure to hold file descriptor set the device is associated with. Have the number written into the Device Info XML and parsed from the XML.
Remember the next-toy use file descriptor set in the QEMU private domain structure. Upon libvirt restart determine the maximum file descriptor set used in the Device Info XML and remember the next-to-use file descriptor set in the QEMU private domain structure.
Upon termination of a domain, reset the next-to-use file descriptor set to its initial value '1'.
Stefan Berger <stefanb@linux.vnet.ibm.com>
--- src/conf/capabilities.h | 5 ++++- src/conf/domain_conf.c | 20 +++++++++++++++++--- src/conf/domain_conf.h | 1 + src/lxc/lxc_domain.c | 3 ++- src/qemu/qemu_domain.c | 23 ++++++++++++++++++++++- src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_process.c | 2 ++ 7 files changed, 50 insertions(+), 6 deletions(-) I really don't like the fact that we're modifying the virDomainDef structs, which are intended to hold static config description, to also hold QEMU specific dynamic state. We did this in the past before we had the per-driver 'privateData' associated with virDomainObjPtr instances.
In these patches we are storing the fdset that's in use by a particular device in the virDomainDeviceInfo structure and are persisting that fdset number next to the device's alias into XML written into /var/run/libvirt/qemu to be able to restore it upon libvirt restart. Now we would either need a similar per-device structure in parallel to those virDomainDeviceInfo structures or connect a QEMU private data structure to that virDomainDeviceInfo structure for storing the dynamic data and to keep reference back to the device. Not sure what's the best path ...
Just have a hashtable mapping device alias names -> fd set numbers in the qemuDomainObjPrivatePtr, and write that out in the QEMU private XML. There's no need to directly mirror the structs.
This is how the per-device XML in /var/run/libvirt/qemu/xyz.xml looks now:
<serial type='pipe'> <source path='/tmp/testpipe'/> <target port='1'/> <alias name='serial1' fdset='3'/> </serial>
Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 02/07/2013 12:58 PM, Daniel P. Berrange wrote:
On Mon, Feb 04, 2013 at 07:12:37PM -0500, Stefan Berger wrote:
Create field in virDomainDeviceInfo structure to hold file descriptor set the device is associated with. Have the number written into the Device Info XML and parsed from the XML. Remember the next-toy use file descriptor set in the QEMU private domain structure. Upon libvirt restart determine the maximum file descriptor set used in the Device Info XML and remember the next-to-use file descriptor set in the QEMU private domain structure.
Upon termination of a domain, reset the next-to-use file descriptor set to its initial value '1'.
Stefan Berger <stefanb@linux.vnet.ibm.com>
--- src/conf/capabilities.h | 5 ++++- src/conf/domain_conf.c | 20 +++++++++++++++++--- src/conf/domain_conf.h | 1 + src/lxc/lxc_domain.c | 3 ++- src/qemu/qemu_domain.c | 23 ++++++++++++++++++++++- src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_process.c | 2 ++ 7 files changed, 50 insertions(+), 6 deletions(-) I really don't like the fact that we're modifying the virDomainDef structs, which are intended to hold static config description, to also hold QEMU specific dynamic state. We did this in the past before we had the per-driver 'privateData' associated with virDomainObjPtr instances. In these patches we are storing the fdset that's in use by a
On 02/07/2013 10:12 AM, Daniel P. Berrange wrote: particular device in the virDomainDeviceInfo structure and are persisting that fdset number next to the device's alias into XML written into /var/run/libvirt/qemu to be able to restore it upon libvirt restart. Now we would either need a similar per-device structure in parallel to those virDomainDeviceInfo structures or connect a QEMU private data structure to that virDomainDeviceInfo structure for storing the dynamic data and to keep reference back to the device. Not sure what's the best path ... Just have a hashtable mapping device alias names -> fd set numbers in the qemuDomainObjPrivatePtr, and write that out in the QEMU
On Thu, Feb 07, 2013 at 12:56:45PM -0500, Stefan Berger wrote: private XML. There's no need to directly mirror the structs.
you agree to this XML ? <fdsets> <entry alias='ide0-0-0' fdset='1'/> <entry alias='serial0' fdset='2'/> <entry alias='serial1' fdset='3'/> </fdsets> Stefan

On 02/07/2013 03:26 PM, Stefan Berger wrote:
Just have a hashtable mapping device alias names -> fd set numbers in the qemuDomainObjPrivatePtr, and write that out in the QEMU private XML. There's no need to directly mirror the structs.
Perhaps my fault for first suggesting that we modify <alias> at the device_conf level, but I like the idea of tracking a hashtable at the qemu private level that maps back to <alias> as needed.
you agree to this XML ?
<fdsets> <entry alias='ide0-0-0' fdset='1'/> <entry alias='serial0' fdset='2'/> <entry alias='serial1' fdset='3'/> </fdsets>
Looks good to me. We may want more information on a given fdset, but that can be added later. If we need more than a alias->set number, we would instead hash an alias->struct, with XML looking something like: <fdsets> <entry alias='ide0-0-0' fdset='1'> <source>/path/to/disk.img</source> <fd id='4' mode='rdonly'/> <fd id='5' mode='rdwr'/> </entry> </fdsets> But again, without knowing whether we need extra information, your proposal is fine for now. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Thu, Feb 07, 2013 at 05:26:10PM -0500, Stefan Berger wrote:
On 02/07/2013 12:58 PM, Daniel P. Berrange wrote:
On Mon, Feb 04, 2013 at 07:12:37PM -0500, Stefan Berger wrote:
Create field in virDomainDeviceInfo structure to hold file descriptor set the device is associated with. Have the number written into the Device Info XML and parsed from the XML. Remember the next-toy use file descriptor set in the QEMU private domain structure. Upon libvirt restart determine the maximum file descriptor set used in the Device Info XML and remember the next-to-use file descriptor set in the QEMU private domain structure.
Upon termination of a domain, reset the next-to-use file descriptor set to its initial value '1'.
Stefan Berger <stefanb@linux.vnet.ibm.com>
--- src/conf/capabilities.h | 5 ++++- src/conf/domain_conf.c | 20 +++++++++++++++++--- src/conf/domain_conf.h | 1 + src/lxc/lxc_domain.c | 3 ++- src/qemu/qemu_domain.c | 23 ++++++++++++++++++++++- src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_process.c | 2 ++ 7 files changed, 50 insertions(+), 6 deletions(-) I really don't like the fact that we're modifying the virDomainDef structs, which are intended to hold static config description, to also hold QEMU specific dynamic state. We did this in the past before we had the per-driver 'privateData' associated with virDomainObjPtr instances. In these patches we are storing the fdset that's in use by a
On 02/07/2013 10:12 AM, Daniel P. Berrange wrote: particular device in the virDomainDeviceInfo structure and are persisting that fdset number next to the device's alias into XML written into /var/run/libvirt/qemu to be able to restore it upon libvirt restart. Now we would either need a similar per-device structure in parallel to those virDomainDeviceInfo structures or connect a QEMU private data structure to that virDomainDeviceInfo structure for storing the dynamic data and to keep reference back to the device. Not sure what's the best path ... Just have a hashtable mapping device alias names -> fd set numbers in the qemuDomainObjPrivatePtr, and write that out in the QEMU
On Thu, Feb 07, 2013 at 12:56:45PM -0500, Stefan Berger wrote: private XML. There's no need to directly mirror the structs.
you agree to this XML ?
<fdsets> <entry alias='ide0-0-0' fdset='1'/> <entry alias='serial0' fdset='2'/> <entry alias='serial1' fdset='3'/> </fdsets>
Yes, sort sort of thing looks better to me Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 02/08/2013 05:22 AM, Daniel P. Berrange wrote:
On Thu, Feb 07, 2013 at 05:26:10PM -0500, Stefan Berger wrote:
you agree to this XML ?
<fdsets> <entry alias='ide0-0-0' fdset='1'/> <entry alias='serial0' fdset='2'/> <entry alias='serial1' fdset='3'/> </fdsets>
Yes, sort sort of thing looks better to me
Great. And v4 came out really good :-) Stefan

Add support for file descriptor sets by converting some of the command line parameters to use /dev/fdset/%d if -add-fd is found to be supported by QEMU. For those devices libvirt now open()s the device to obtain the file descriptor and 'transfers' the fd using virCommand. For the following fragments of domain XML <disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/var/lib/libvirt/images/fc14-x86_64.img'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <serial type='dev'> <source path='/dev/ttyS0'/> <target port='0'/> </serial> <serial type='pipe'> <source path='/tmp/testpipe'/> <target port='1'/> </serial> libvirt now creates the following parts for the QEMU command line: old: -drive file=/var/lib/libvirt/images/fc14-x86_64.img,if=none,id=drive-ide0-0-0,format=raw new: -add-fd set=1,fd=23,opaque=RDONLY:/var/lib/libvirt/images/fc14-x86_64.img -add-fd set=1,fd=24,opaque=RDWR:/var/lib/libvirt/images/fc14-x86_64.img -drive file=/dev/fdset/1,if=none,id=drive-ide0-0-0,format=raw old: -chardev tty,id=charserial0,path=/dev/ttyS0 new: -add-fd set=1,fd=30,opaque=/dev/ttyS0 -chardev tty,id=charserial0,path=/dev/fdset/1 old: -chardev pipe,id=charserial1,path=/tmp/testpipe new: -add-fd set=2,fd=32,opaque=/tmp/testpipe -chardev pipe,id=charserial1,path=/dev/fdset/2 Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- v2->v3: - Use an unsigned int for remembering the last used fdset v1->v2: - Addressed many of Eric's comment; many changes, though - virBitmap holds used file descriptor sets; it's attached to QEMU private domain structure - persisting and parsing the fdset in the virDomainDeviceInfo XML - rebuilding the fdset bitmap upon libvirt start and after parsing the virDomainDeviceInfo XML --- src/qemu/qemu_command.c | 238 ++++++++++++++++++++++++++++++++++++++--------- src/qemu/qemu_command.h | 15 ++ src/qemu/qemu_driver.c | 5 src/qemu/qemu_driver.h | 4 src/qemu/qemu_hotplug.c | 9 + src/qemu/qemu_process.c | 3 tests/qemuxml2argvtest.c | 3 tests/qemuxmlnstest.c | 3 8 files changed, 224 insertions(+), 56 deletions(-) Index: libvirt/src/qemu/qemu_command.c =================================================================== --- libvirt.orig/src/qemu/qemu_command.c +++ libvirt/src/qemu/qemu_command.c @@ -46,6 +46,7 @@ #include "base64.h" #include "device_conf.h" #include "virstoragefile.h" +#include "qemu_driver.h" #include <sys/stat.h> #include <fcntl.h> @@ -133,6 +134,70 @@ VIR_ENUM_IMPL(qemuDomainFSDriver, VIR_DO /** + * qemuCommandPrintFDSet: + * @fdset: the number of the file descriptor set to which @fd belongs + * @fd: fd that will be assigned to QEMU + * @open_flags: the flags used for opening the fd; of interest are only + * O_RDONLY, O_WRONLY, O_RDWR + * @name: optional name of the file + * + * Write the parameters for the QEMU -add-fd command line option + * for the given file descriptor and return the string. + * This function for example returns "set=10,fd=20,opaque=RDWR:/foo/bar". + */ +static char * +qemuCommandPrintFDSet(int fdset, int fd, int open_flags, const char *name) +{ + const char *mode = ""; + virBuffer buf = VIR_BUFFER_INITIALIZER; + + if (name) { + switch ((open_flags & O_ACCMODE)) { + case O_RDONLY: + mode = "RDONLY:"; + break; + case O_WRONLY: + mode = "WRONLY:"; + break; + case O_RDWR: + mode = "RDWR:"; + break; + } + } + + virBufferAsprintf(&buf, "set=%d,fd=%d%s%s", fdset, fd, + (name) ? ",opaque=" : "", + mode); + if (name) + virBufferEscape(&buf, ',', ",", "%s", name); + + if (virBufferError(&buf)) { + virReportOOMError(); + virBufferFreeAndReset(&buf); + return NULL; + } + + return virBufferContentAndReset(&buf); +} + +/** + * qemuCommandPrintDevSet: + * @buf: buffer to write the file descriptor set string + * @fdset: the file descriptor set + * + * Get the parameters for the QEMU path= parameter where a file + * descriptor is accessed via a file descriptor set, for example + * /dev/fdset/10. The file descriptor must previously have been + * 'transferred' in a virCommandTransfer() call. + */ +static void +qemuCommandPrintDevSet(virBufferPtr buf, int fdset) +{ + virBufferAsprintf(buf, "/dev/fdset/%d", fdset); +} + + +/** * qemuPhysIfaceConnect: * @def: the definition of the VM (needed by 802.1Qbh and audit) * @driver: pointer to the driver instance @@ -2223,11 +2288,66 @@ no_memory: goto cleanup; } +static int +qemuCreatePathForFilePath(virQEMUDriverPtr driver, virBufferPtr buf, + const char *prefix, const char *path, + const int *open_flags, + virCommandPtr cmd, unsigned int *nextfdset, + unsigned int *fdsetnum, + qemuCapsPtr caps) +{ + char *fdsetstr = NULL; + int i; + + virBufferAdd(buf, prefix, -1); + /* + * cmd == NULL hints at hotplugging; use old way of doing things + * nextfdset == NULL hints at a call path where we should not open files + * In this case we fall back to the old command line + * (at least for now) + */ + if (!cmd || !nextfdset || !qemuCapsGet(caps, QEMU_CAPS_ADD_FD)) { + virBufferEscape(buf, ',', ",", "%s", path); + } else { + *fdsetnum = (*nextfdset)++; + + qemuCommandPrintDevSet(buf, *fdsetnum); + + i = 0; + while (open_flags[i] != -1) { + int fd = qemuOpenFile(driver, path, open_flags[i], NULL, NULL); + if (fd < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not open %s"), path); + goto error; + } + virCommandTransferFD(cmd, fd); + + fdsetstr = qemuCommandPrintFDSet(*fdsetnum, fd, open_flags[i], + path); + if (!fdsetstr) + goto error; + virCommandAddArgList(cmd, "-add-fd", fdsetstr, NULL); + VIR_FREE(fdsetstr); + + i++; + } + } + + return 0; + +error: + return -1; +} + char * qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainDiskDefPtr disk, bool bootable, - qemuCapsPtr caps) + qemuCapsPtr caps, + virCommandPtr cmd, + virQEMUDriverPtr driver, + unsigned int *nextfdset) { virBuffer opt = VIR_BUFFER_INITIALIZER; const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus); @@ -2389,7 +2509,13 @@ qemuBuildDriveStr(virConnectPtr conn ATT "block type disk")); goto error; } - virBufferEscape(&opt, ',', ",", "file=%s,", disk->src); + if (qemuCreatePathForFilePath(driver, &opt, + "file=", disk->src, + (int[]){O_RDONLY, O_RDWR, -1}, + cmd, nextfdset, &disk->info.fdset, + caps) < 0) + goto error; + virBufferAddChar(&opt, ','); } } if (qemuCapsGet(caps, QEMU_CAPS_DEVICE)) @@ -2886,7 +3012,10 @@ error: char *qemuBuildFSStr(virDomainFSDefPtr fs, - qemuCapsPtr caps ATTRIBUTE_UNUSED) + qemuCapsPtr caps ATTRIBUTE_UNUSED, + virCommandPtr cmd, + virQEMUDriverPtr qemu_driver, + unsigned int *nextfdset) { virBuffer opt = VIR_BUFFER_INITIALIZER; const char *driver = qemuDomainFSDriverTypeToString(fs->fsdriver); @@ -2935,7 +3064,10 @@ char *qemuBuildFSStr(virDomainFSDefPtr f } virBufferAsprintf(&opt, ",id=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); - virBufferAsprintf(&opt, ",path=%s", fs->src); + if (qemuCreatePathForFilePath(qemu_driver, &opt, + ",path=", fs->src, (int[]){O_RDWR, -1}, + cmd, nextfdset, &fs->info.fdset, caps) < 0) + goto error; if (fs->readonly) { if (qemuCapsGet(caps, QEMU_CAPS_FSDEV_READONLY)) { @@ -3884,12 +4016,13 @@ qemuBuildUSBHostdevUsbDevStr(virDomainHo } - /* This function outputs a -chardev command line option which describes only the * host side of the character device */ static char * qemuBuildChrChardevStr(virDomainChrSourceDefPtr dev, const char *alias, - qemuCapsPtr caps) + qemuCapsPtr caps, virCommandPtr cmd, + virQEMUDriverPtr driver, + unsigned int *nextfdset, unsigned int *fdsetnum) { virBuffer buf = VIR_BUFFER_INITIALIZER; bool telnet; @@ -3908,19 +4041,32 @@ qemuBuildChrChardevStr(virDomainChrSourc break; case VIR_DOMAIN_CHR_TYPE_DEV: - virBufferAsprintf(&buf, "%s,id=char%s,path=%s", + virBufferAsprintf(&buf, "%s,id=char%s", STRPREFIX(alias, "parallel") ? "parport" : "tty", - alias, dev->data.file.path); + alias); + if (qemuCreatePathForFilePath(driver, &buf, + ",path=", dev->data.file.path, + (int[]){O_RDWR, -1}, cmd, nextfdset, + fdsetnum, caps) < 0) + goto error; break; case VIR_DOMAIN_CHR_TYPE_FILE: - virBufferAsprintf(&buf, "file,id=char%s,path=%s", alias, - dev->data.file.path); + virBufferAsprintf(&buf, "file,id=char%s", alias); + if (qemuCreatePathForFilePath(driver, &buf, + ",path=", dev->data.file.path, + (int[]){O_RDWR, -1}, cmd, nextfdset, + fdsetnum, caps) < 0) + goto error; break; case VIR_DOMAIN_CHR_TYPE_PIPE: - virBufferAsprintf(&buf, "pipe,id=char%s,path=%s", alias, - dev->data.file.path); + virBufferAsprintf(&buf, "pipe,id=char%s",alias); + if (qemuCreatePathForFilePath(driver, &buf, + ",path=", dev->data.file.path, + (int[]){O_RDWR, -1}, cmd, + nextfdset, fdsetnum, caps) < 0) + goto error; break; case VIR_DOMAIN_CHR_TYPE_STDIO: @@ -5056,7 +5202,8 @@ qemuBuildCommandLine(virConnectPtr conn, const char *migrateFrom, int migrateFd, virDomainSnapshotObjPtr snapshot, - enum virNetDevVPortProfileOp vmop) + enum virNetDevVPortProfileOp vmop, + unsigned int *nextfdset) { int i, j; int disableKQEMU = 0; @@ -5350,11 +5497,11 @@ qemuBuildCommandLine(virConnectPtr conn, /* Use -chardev if it's available */ if (qemuCapsGet(caps, QEMU_CAPS_CHARDEV)) { - virCommandAddArg(cmd, "-chardev"); if (!(chrdev = qemuBuildChrChardevStr(monitor_chr, "monitor", - caps))) + caps, cmd, driver, nextfdset, + NULL))) goto error; - virCommandAddArg(cmd, chrdev); + virCommandAddArgList(cmd, "-chardev", chrdev, NULL); VIR_FREE(chrdev); virCommandAddArg(cmd, "-mon"); @@ -5797,8 +5944,6 @@ qemuBuildCommandLine(virConnectPtr conn, break; } - virCommandAddArg(cmd, "-drive"); - /* Unfortunately it is not possible to use -device for floppies, or Xen paravirt devices. Fortunately, those don't need @@ -5814,12 +5959,12 @@ qemuBuildCommandLine(virConnectPtr conn, } optstr = qemuBuildDriveStr(conn, disk, emitBootindex ? false : !!bootindex, - caps); + caps, cmd, driver, nextfdset); if (deviceFlagMasked) qemuCapsSet(caps, QEMU_CAPS_DEVICE); if (!optstr) goto error; - virCommandAddArg(cmd, optstr); + virCommandAddArgList(cmd, "-drive", optstr, NULL); VIR_FREE(optstr); if (!emitBootindex) @@ -5995,7 +6140,7 @@ qemuBuildCommandLine(virConnectPtr conn, virDomainFSDefPtr fs = def->fss[i]; virCommandAddArg(cmd, "-fsdev"); - if (!(optstr = qemuBuildFSStr(fs, caps))) + if (!(optstr = qemuBuildFSStr(fs, caps, cmd, driver, nextfdset))) goto error; virCommandAddArg(cmd, optstr); VIR_FREE(optstr); @@ -6275,14 +6420,14 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(&smartcard->data.passthru, smartcard->info.alias, - caps))) { + caps, cmd, driver, nextfdset, + &smartcard->info.fdset))) { virBufferFreeAndReset(&opt); goto error; } - virCommandAddArg(cmd, devstr); + virCommandAddArgList(cmd, "-chardev", devstr, NULL); VIR_FREE(devstr); virBufferAsprintf(&opt, "ccid-card-passthru,chardev=char%s", @@ -6313,12 +6458,13 @@ qemuBuildCommandLine(virConnectPtr conn, /* Use -chardev with -device if they are available */ if (qemuCapsGet(caps, QEMU_CAPS_CHARDEV) && qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { - virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(&serial->source, serial->info.alias, - caps))) + caps, cmd, driver, + nextfdset, + &serial->info.fdset))) goto error; - virCommandAddArg(cmd, devstr); + virCommandAddArgList(cmd, "-chardev", devstr, NULL); VIR_FREE(devstr); virCommandAddArg(cmd, "-device"); @@ -6350,12 +6496,13 @@ qemuBuildCommandLine(virConnectPtr conn, /* Use -chardev with -device if they are available */ if (qemuCapsGet(caps, QEMU_CAPS_CHARDEV) && qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { - virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(¶llel->source, parallel->info.alias, - caps))) + caps, cmd, driver, + nextfdset, + ¶llel->info.fdset))) goto error; - virCommandAddArg(cmd, devstr); + virCommandAddArgList(cmd, "-chardev", devstr, NULL); VIR_FREE(devstr); virCommandAddArg(cmd, "-device"); @@ -6387,12 +6534,12 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(&channel->source, channel->info.alias, - caps))) + caps, cmd, driver, nextfdset, + &channel->info.fdset))) goto error; - virCommandAddArg(cmd, devstr); + virCommandAddArgList(cmd, "-chardev", devstr, NULL); VIR_FREE(devstr); addr = virSocketAddrFormat(channel->target.addr); @@ -6422,12 +6569,13 @@ qemuBuildCommandLine(virConnectPtr conn, * the newer -chardev interface. */ ; } else { - virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(&channel->source, channel->info.alias, - caps))) + caps, cmd, driver, + nextfdset, + &channel->info.fdset))) goto error; - virCommandAddArg(cmd, devstr); + virCommandAddArgList(cmd, "-chardev", devstr, NULL); VIR_FREE(devstr); } @@ -6460,12 +6608,12 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(&console->source, console->info.alias, - caps))) + caps, cmd, driver, nextfdset, + &console->info.fdset))) goto error; - virCommandAddArg(cmd, devstr); + virCommandAddArgList(cmd, "-chardev", devstr, NULL); VIR_FREE(devstr); virCommandAddArg(cmd, "-device"); @@ -6482,12 +6630,12 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(&console->source, console->info.alias, - caps))) + caps, cmd, driver, nextfdset, + &console->info.fdset))) goto error; - virCommandAddArg(cmd, devstr); + virCommandAddArgList(cmd, "-chardev", devstr, NULL); VIR_FREE(devstr); virCommandAddArg(cmd, "-device"); @@ -6824,14 +6972,14 @@ qemuBuildCommandLine(virConnectPtr conn, virDomainRedirdevDefPtr redirdev = def->redirdevs[i]; char *devstr; - virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(&redirdev->source.chr, redirdev->info.alias, - caps))) { + caps, cmd, driver, nextfdset, + &redirdev->info.fdset))) { goto error; } - virCommandAddArg(cmd, devstr); + virCommandAddArgList(cmd, "-chardev", devstr, NULL); VIR_FREE(devstr); if (!qemuCapsGet(caps, QEMU_CAPS_DEVICE)) Index: libvirt/src/qemu/qemu_command.h =================================================================== --- libvirt.orig/src/qemu/qemu_command.h +++ libvirt/src/qemu/qemu_command.h @@ -30,6 +30,7 @@ # include "qemu_conf.h" # include "qemu_domain.h" # include "qemu_capabilities.h" +# include "virbitmap.h" /* Config type for XML import/export conversions */ # define QEMU_CONFIG_FORMAT_ARGV "qemu-argv" @@ -58,7 +59,8 @@ virCommandPtr qemuBuildCommandLine(virCo const char *migrateFrom, int migrateFd, virDomainSnapshotObjPtr current_snapshot, - enum virNetDevVPortProfileOp vmop) + enum virNetDevVPortProfileOp vmop, + unsigned int *nextfdset) ATTRIBUTE_NONNULL(1); /* Generate string for arch-specific '-device' parameter */ @@ -95,9 +97,16 @@ char *qemuDeviceDriveHostAlias(virDomain char *qemuBuildDriveStr(virConnectPtr conn, virDomainDiskDefPtr disk, bool bootable, - qemuCapsPtr caps); + qemuCapsPtr caps, + virCommandPtr cmd, + virQEMUDriverPtr driver, + unsigned int *nextfdset); + char *qemuBuildFSStr(virDomainFSDefPtr fs, - qemuCapsPtr caps); + qemuCapsPtr caps, + virCommandPtr cmd, + virQEMUDriverPtr driver, + unsigned int *nextfdset); /* Current, best practice */ char * qemuBuildDriveDevStr(virDomainDefPtr def, Index: libvirt/src/qemu/qemu_driver.c =================================================================== --- libvirt.orig/src/qemu/qemu_driver.c +++ libvirt/src/qemu/qemu_driver.c @@ -2679,7 +2679,7 @@ qemuCompressProgramName(int compress) /* Internal function to properly create or open existing files, with * ownership affected by qemu driver setup. */ -static int +int qemuOpenFile(virQEMUDriverPtr driver, const char *path, int oflags, bool *needUnlink, bool *bypassSecurityDriver) { @@ -5437,7 +5437,8 @@ static char *qemuDomainXMLToNative(virCo if (!(cmd = qemuBuildCommandLine(conn, driver, def, &monConfig, monitor_json, caps, - NULL, -1, NULL, VIR_NETDEV_VPORT_PROFILE_OP_NO_OP))) + NULL, -1, NULL, VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, + NULL))) goto cleanup; ret = virCommandToString(cmd); Index: libvirt/src/qemu/qemu_hotplug.c =================================================================== --- libvirt.orig/src/qemu/qemu_hotplug.c +++ libvirt/src/qemu/qemu_hotplug.c @@ -262,7 +262,8 @@ int qemuDomainAttachPciDiskDevice(virCon if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->caps) < 0) goto error; - if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->caps))) + if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->caps, + NULL, driver, &priv->nextfdset))) goto error; if (!(devstr = qemuBuildDriveDevStr(NULL, disk, 0, priv->caps))) @@ -503,7 +504,8 @@ int qemuDomainAttachSCSIDisk(virConnectP goto error; } - if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->caps))) + if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->caps, NULL, + driver, &priv->nextfdset))) goto error; for (i = 0 ; i <= disk->info.addr.drive.controller ; i++) { @@ -622,7 +624,8 @@ int qemuDomainAttachUsbMassstorageDevice if (qemuCapsGet(priv->caps, QEMU_CAPS_DEVICE)) { if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->caps) < 0) goto error; - if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->caps))) + if (!(drivestr = qemuBuildDriveStr(conn, disk, false, priv->caps, + NULL, driver, &priv->nextfdset))) goto error; if (!(devstr = qemuBuildDriveDevStr(NULL, disk, 0, priv->caps))) goto error; Index: libvirt/src/qemu/qemu_process.c =================================================================== --- libvirt.orig/src/qemu/qemu_process.c +++ libvirt/src/qemu/qemu_process.c @@ -3779,7 +3779,8 @@ int qemuProcessStart(virConnectPtr conn, VIR_DEBUG("Building emulator command line"); if (!(cmd = qemuBuildCommandLine(conn, driver, vm->def, priv->monConfig, priv->monJSON != 0, priv->caps, - migrateFrom, stdin_fd, snapshot, vmop))) + migrateFrom, stdin_fd, snapshot, vmop, + &priv->nextfdset))) goto cleanup; /* now that we know it is about to start call the hook if present */ Index: libvirt/tests/qemuxml2argvtest.c =================================================================== --- libvirt.orig/tests/qemuxml2argvtest.c +++ libvirt/tests/qemuxml2argvtest.c @@ -151,7 +151,8 @@ static int testCompareXMLToArgvFiles(con if (!(cmd = qemuBuildCommandLine(conn, &driver, vmdef, &monitor_chr, (flags & FLAG_JSON), extraFlags, migrateFrom, migrateFd, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_NO_OP))) { + VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, + NULL))) { if (flags & FLAG_EXPECT_FAILURE) { ret = 0; virResetLastError(); Index: libvirt/tests/qemuxmlnstest.c =================================================================== --- libvirt.orig/tests/qemuxmlnstest.c +++ libvirt/tests/qemuxmlnstest.c @@ -110,7 +110,8 @@ static int testCompareXMLToArgvFiles(con if (!(cmd = qemuBuildCommandLine(conn, &driver, vmdef, &monitor_chr, json, extraFlags, migrateFrom, migrateFd, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_NO_OP))) + VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, + NULL))) goto fail; if (!!virGetLastError() != expectError) { Index: libvirt/src/qemu/qemu_driver.h =================================================================== --- libvirt.orig/src/qemu/qemu_driver.h +++ libvirt/src/qemu/qemu_driver.h @@ -24,6 +24,10 @@ #ifndef __QEMU_DRIVER_H__ # define __QEMU_DRIVER_H__ +# include "qemu_conf.h" + int qemuRegister(void); +int qemuOpenFile(virQEMUDriverPtr driver, const char *path, int oflags, + bool *needUnlink, bool *bypassSecurityDriver); #endif /* __QEMU_DRIVER_H__ */

On Mon, Feb 04, 2013 at 07:12:38PM -0500, Stefan Berger wrote:
Add support for file descriptor sets by converting some of the command line parameters to use /dev/fdset/%d if -add-fd is found to be supported by QEMU. For those devices libvirt now open()s the device to obtain the file descriptor and 'transfers' the fd using virCommand.
For the following fragments of domain XML
<disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/var/lib/libvirt/images/fc14-x86_64.img'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk>
<serial type='dev'> <source path='/dev/ttyS0'/> <target port='0'/> </serial> <serial type='pipe'> <source path='/tmp/testpipe'/> <target port='1'/> </serial>
libvirt now creates the following parts for the QEMU command line:
old: -drive file=/var/lib/libvirt/images/fc14-x86_64.img,if=none,id=drive-ide0-0-0,format=raw new: -add-fd set=1,fd=23,opaque=RDONLY:/var/lib/libvirt/images/fc14-x86_64.img -add-fd set=1,fd=24,opaque=RDWR:/var/lib/libvirt/images/fc14-x86_64.img -drive file=/dev/fdset/1,if=none,id=drive-ide0-0-0,format=raw
old: -chardev tty,id=charserial0,path=/dev/ttyS0 new: -add-fd set=1,fd=30,opaque=/dev/ttyS0 -chardev tty,id=charserial0,path=/dev/fdset/1
old: -chardev pipe,id=charserial1,path=/tmp/testpipe new: -add-fd set=2,fd=32,opaque=/tmp/testpipe -chardev pipe,id=charserial1,path=/dev/fdset/2
Given this significant change to the ARGV generation, I'd expect to see more than just this change in the test suite:
Index: libvirt/tests/qemuxml2argvtest.c =================================================================== --- libvirt.orig/tests/qemuxml2argvtest.c +++ libvirt/tests/qemuxml2argvtest.c @@ -151,7 +151,8 @@ static int testCompareXMLToArgvFiles(con if (!(cmd = qemuBuildCommandLine(conn, &driver, vmdef, &monitor_chr, (flags & FLAG_JSON), extraFlags, migrateFrom, migrateFd, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_NO_OP))) { + VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, + NULL))) { if (flags & FLAG_EXPECT_FAILURE) { ret = 0; virResetLastError(); Index: libvirt/tests/qemuxmlnstest.c =================================================================== --- libvirt.orig/tests/qemuxmlnstest.c +++ libvirt/tests/qemuxmlnstest.c @@ -110,7 +110,8 @@ static int testCompareXMLToArgvFiles(con if (!(cmd = qemuBuildCommandLine(conn, &driver, vmdef, &monitor_chr, json, extraFlags, migrateFrom, migrateFd, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_NO_OP))) + VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, + NULL))) goto fail;
if (!!virGetLastError() != expectError) {
Yes, this nicely shows that you didn't break any existing QEMU ARGV generation, but at the same time it shows there's zero testing coverage of the new code Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 02/07/2013 10:14 AM, Daniel P. Berrange wrote:
On Mon, Feb 04, 2013 at 07:12:38PM -0500, Stefan Berger wrote:
Add support for file descriptor sets by converting some of the command line parameters to use /dev/fdset/%d if -add-fd is found to be supported by QEMU. For those devices libvirt now open()s the device to obtain the file descriptor and 'transfers' the fd using virCommand.
For the following fragments of domain XML
<disk type='file' device='disk'> <driver name='qemu' type='raw'/> <source file='/var/lib/libvirt/images/fc14-x86_64.img'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk>
<serial type='dev'> <source path='/dev/ttyS0'/> <target port='0'/> </serial> <serial type='pipe'> <source path='/tmp/testpipe'/> <target port='1'/> </serial>
libvirt now creates the following parts for the QEMU command line:
old: -drive file=/var/lib/libvirt/images/fc14-x86_64.img,if=none,id=drive-ide0-0-0,format=raw new: -add-fd set=1,fd=23,opaque=RDONLY:/var/lib/libvirt/images/fc14-x86_64.img -add-fd set=1,fd=24,opaque=RDWR:/var/lib/libvirt/images/fc14-x86_64.img -drive file=/dev/fdset/1,if=none,id=drive-ide0-0-0,format=raw
old: -chardev tty,id=charserial0,path=/dev/ttyS0 new: -add-fd set=1,fd=30,opaque=/dev/ttyS0 -chardev tty,id=charserial0,path=/dev/fdset/1
old: -chardev pipe,id=charserial1,path=/tmp/testpipe new: -add-fd set=2,fd=32,opaque=/tmp/testpipe -chardev pipe,id=charserial1,path=/dev/fdset/2 Given this significant change to the ARGV generation, I'd expect to see more than just this change in the test suite:
Which devices can we assume to be using in the test suite? I guess /dev/null would be a safe bet to not touch anything critical on a system. I'd add a patch on top of this series providing coverage. Stefan
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Stefan Berger