[libvirt] [PATCH V6 3/3] Add support for file descriptor sets

Corey Bryant coreyb at linux.vnet.ibm.com
Fri Feb 15 20:51:25 UTC 2013



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 at linux.vnet.ibm.com>
>
> ---
> 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




More information about the libvir-list mailing list