On 02/14/2013 07:00 AM, 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
Test cases are part of this patch now.
Signed-off-by: Stefan Berger<stefanb@xxxxxxxxxxxxxxxxxx>
---
v4->v5:
- removed one test case testing for 'old' command line
v3->v4:
- Adapt to changes in patch 1
- better handling of error case on the hotplug code
v2->v3:
- Use an unsigned int for remembering the next-to-use 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 | 243 +++++++++++++++++++-----
src/qemu/qemu_command.h | 14 +
src/qemu/qemu_driver.c | 5
src/qemu/qemu_driver.h | 4
src/qemu/qemu_hotplug.c | 15 +
src/qemu/qemu_process.c | 4
tests/qemuxml2argvdata/qemuxml2argv-add-fd.args | 23 ++
tests/qemuxml2argvdata/qemuxml2argv-add-fd.xml | 36 +++
tests/qemuxml2argvtest.c | 11 -
tests/qemuxmlnstest.c | 8
10 files changed, 308 insertions(+), 55 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)
It would be easier to read the code if this were called something like
qemuCommandPrintAddFd().
+{
+ 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)
And this could be called qemuCommandPrintDevFdSet().
+{
+ 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
@@ -2229,11 +2294,70 @@ no_memory:
goto cleanup;
}
+static int
+qemuCreatePathForFilePath(virQEMUDriverPtr driver, virBufferPtr buf,
+ const char *prefix, const char *path,
+ const int *open_flags,
+ virCommandPtr cmd, virFdSetPtr fdset,
+ const virDomainDeviceInfoPtr info,
+ virQEMUCapsPtr qemuCaps)
+{
+ char *fdsetstr = NULL;
+ int i;
+
+ virBufferAdd(buf, prefix, -1);
+ /*
+ * cmd == NULL hints at hotplugging; use old way of doing things
I assume hotplug will be implemented at a later time by someone.
+ * fdset == 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 || !fdset || !virQEMUCapsGet(qemuCaps, QEMU_CAPS_ADD_FD)) {
+ virBufferEscape(buf, ',', ",", "%s", path);
+ } else {
+ unsigned int fdsetnum;
+
+ if (virFdSetNextSet(fdset, info->alias, &fdsetnum) < 0)
+ goto error;
+
+ 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);
And fdsetstr could be called addfdstr?
+ if (!fdsetstr)
+ goto error;
+ virCommandAddArgList(cmd, "-add-fd", fdsetstr, NULL);
+ VIR_FREE(fdsetstr);
+
+ i++;
+ }
+ }
+
+ return 0;
+
+error:
Do fd's need to get cleaned up? Are we leaking them here?
+ virFdSetRemoveAlias(fdset, info->alias);
+ return -1;
+}
+
--
Regards,
Corey Bryant
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list