[libvirt] [PATCH 0/6 v3] Support NBD volumes with LXC containers
From: "Daniel P. Berrange" <berrange@redhat.com> A third version of: https://www.redhat.com/archives/libvir-list/2013-April/msg01539.html In v3 - Split patch 3 into 2 pieces - Added XML formatting tests - Fixed XML formatting - Remove !! from vars that are now bools Daniel P. Berrange (6): Add support for storage format in FS <driver> Re-arrange code setting up ifs/disk loop devices for LXC Fix error handling of readdir() in virFileLoopDeviceOpen Add a helper API for setting up a NBD device with qemu-nbd Add 'nbd' as a valid filesystem driver type Support NBD backed disks/filesystems in LXC driver docs/formatdomain.html.in | 25 +++++ docs/schemas/domaincommon.rng | 74 +++++++++++---- src/conf/domain_conf.c | 33 +++++-- src/conf/domain_conf.h | 7 +- src/libvirt_private.syms | 1 + src/lxc/lxc_controller.c | 152 ++++++++++++++++++++++------- src/qemu/qemu_command.c | 4 +- src/util/virfile.c | 153 +++++++++++++++++++++++++++++- src/util/virfile.h | 6 ++ tests/lxcxml2xmldata/lxc-disk-formats.xml | 31 ++++++ tests/lxcxml2xmltest.c | 1 + 11 files changed, 421 insertions(+), 66 deletions(-) create mode 100644 tests/lxcxml2xmldata/lxc-disk-formats.xml -- 1.8.2.1
From: "Daniel P. Berrange" <berrange@redhat.com> Extend the <driver> element in filesystem devices to allow a storage format to be set. The new attribute uses 'format' to reflect the storage format. This is different from the <driver> element in disk devices which use 'type' to reflect the storage format. This is because the 'type' attribute on filesystem devices is already used for the driver backend, for which the disk devices use the 'name' attribute. Arggggh. Anyway for disks we have <driver name="qemu" type="raw"/> And for filesystems this change means we now have <driver type="loop" format="raw"/> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- docs/formatdomain.html.in | 24 ++++++++++ docs/schemas/domaincommon.rng | 73 ++++++++++++++++++++++--------- src/conf/domain_conf.c | 32 +++++++++++--- src/conf/domain_conf.h | 6 ++- src/qemu/qemu_command.c | 3 +- tests/lxcxml2xmldata/lxc-disk-formats.xml | 26 +++++++++++ tests/lxcxml2xmltest.c | 1 + 7 files changed, 134 insertions(+), 31 deletions(-) create mode 100644 tests/lxcxml2xmldata/lxc-disk-formats.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3570bbc..c95756e 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1876,6 +1876,13 @@ <target dir='/import/from/host'/> <readonly/> </filesystem> + <filesystem type='file' accessmode='passthrough'> + <driver name='loop' type='raw'/> + <driver type='path' wrpolicy='immediate'/> + <source file='/export/to/guest.img'/> + <target dir='/import/from/host'/> + <readonly/> + </filesystem> ... </devices> ...</pre> @@ -1967,6 +1974,23 @@ </dd> + <dt><code>driver</code></dt> + <dd> + The optional driver element allows specifying further details + related to the hypervisor driver used to provide the filesystem. + <span class="since">Since 1.0.6</span> + <ul> + <li> + If the hypervisor supports multiple backend drivers, then + the <code>type</code> attribute selects the primary + backend driver name, while the <code>format</code> + attribute provides the format type. For example, LXC + supports a type of "loop", with a format of "raw". QEMU + supports a type of "path" or "handle", but no formats. + </li> + </ul> + </dd> + <dt><code>source</code></dt> <dd> The resource on the host that is being accessed in the guest. The diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 10596dc..7d1cfa6 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -888,7 +888,7 @@ <define name="diskspec"> <interleave> <optional> - <ref name="driver"/> + <ref name="diskDriver"/> </optional> <optional> <ref name='diskMirror'/> @@ -1270,7 +1270,7 @@ <!-- Disk may use a special driver for access. --> - <define name="driver"> + <define name="diskDriver"> <element name="driver"> <choice> <group> @@ -1314,13 +1314,13 @@ <optional> <attribute name='type'> <choice> - <ref name='diskFormat'/> + <ref name='storageFormat'/> <value>aio</value> <!-- back-compat for 'raw' --> </choice> </attribute> </optional> </define> - <define name='diskFormat'> + <define name='storageFormat'> <choice> <value>raw</value> <value>dir</value> @@ -1518,6 +1518,9 @@ <attribute name="type"> <value>file</value> </attribute> + <optional> + <ref name="diskDriver"/> + </optional> <interleave> <element name="source"> <attribute name="file"> @@ -1531,6 +1534,9 @@ <attribute name="type"> <value>block</value> </attribute> + <optional> + <ref name="diskDriver"/> + </optional> <interleave> <element name="source"> <attribute name="dev"> @@ -1547,6 +1553,9 @@ <value>mount</value> </attribute> </optional> + <optional> + <ref name="diskDriver"/> + </optional> <interleave> <element name="source"> <attribute name="dir"> @@ -1554,22 +1563,6 @@ </attribute> <empty/> </element> - <optional> - <element name="driver"> - <attribute name="type"> - <choice> - <value>path</value> - <value>handle</value> - </choice> - </attribute> - <optional> - <attribute name="wrpolicy"> - <value>immediate</value> - </attribute> - </optional> - <empty/> - </element> - </optional> </interleave> </group> <group> @@ -1578,6 +1571,9 @@ <value>bind</value> </attribute> </optional> + <optional> + <ref name="diskDriver"/> + </optional> <interleave> <element name="source"> <attribute name="dir"> @@ -1591,6 +1587,9 @@ <attribute name="type"> <value>template</value> </attribute> + <optional> + <ref name="diskDriver"/> + </optional> <interleave> <element name="source"> <attribute name="name"> @@ -1604,6 +1603,9 @@ <attribute name="type"> <value>ram</value> </attribute> + <optional> + <ref name="diskDriver"/> + </optional> <interleave> <element name="source"> <attribute name="usage"> @@ -1661,6 +1663,35 @@ </interleave> </element> </define> + <define name="fsDriver"> + <element name="driver"> + <!-- Annoying inconsistency. 'disk' uses 'name' + for this kind of info, and 'type' for the + storage format. We need the latter too, so + had to invent a new attribute name --> + <optional> + <attribute name="type"> + <choice> + <value>path</value> + <value>handle</value> + <value>loop</value> + </choice> + </attribute> + </optional> + <optional> + <attribute name="format"> + <ref name="storageFormat"/> + </attribute> + </optional> + <optional> + <attribute name="wrpolicy"> + <value>immediate</value> + </attribute> + </optional> + <empty/> + </element> + </define> + <!-- An interface description can either be of type bridge in which case it will use a bridging source, or of type ethernet which uses a device @@ -3805,7 +3836,7 @@ </attribute> <optional> <attribute name='format'> - <ref name='diskFormat'/> + <ref name='storageFormat'/> </attribute> </optional> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fe97c02..f858485 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -342,7 +342,8 @@ VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST, VIR_ENUM_IMPL(virDomainFSDriverType, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, "default", "path", - "handle") + "handle", + "loop") VIR_ENUM_IMPL(virDomainFSAccessMode, VIR_DOMAIN_FS_ACCESSMODE_LAST, "passthrough", @@ -5495,6 +5496,7 @@ virDomainFSDefParseXML(xmlNodePtr node, char *fsdriver = NULL; char *source = NULL; char *target = NULL; + char *format = NULL; char *accessmode = NULL; char *wrpolicy = NULL; char *usage = NULL; @@ -5563,9 +5565,13 @@ virDomainFSDefParseXML(xmlNodePtr node, target = virXMLPropString(cur, "dir"); } else if (xmlStrEqual(cur->name, BAD_CAST "readonly")) { def->readonly = true; - } else if (!fsdriver && xmlStrEqual(cur->name, BAD_CAST "driver")) { - fsdriver = virXMLPropString(cur, "type"); - wrpolicy = virXMLPropString(cur, "wrpolicy"); + } else if (xmlStrEqual(cur->name, BAD_CAST "driver")) { + if (!fsdriver) + fsdriver = virXMLPropString(cur, "type"); + if (!wrpolicy) + wrpolicy = virXMLPropString(cur, "wrpolicy"); + if (!format) + format = virXMLPropString(cur, "format"); } } cur = cur->next; @@ -5573,12 +5579,20 @@ virDomainFSDefParseXML(xmlNodePtr node, if (fsdriver) { if ((def->fsdriver = virDomainFSDriverTypeTypeFromString(fsdriver)) <= 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown fs driver type '%s'"), fsdriver); goto error; } } + if (format) { + if ((def->format = virStorageFileFormatTypeFromString(format)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown driver format value '%s'"), format); + goto error; + } + } + if (wrpolicy) { if ((def->wrpolicy = virDomainFSWrpolicyTypeFromString(wrpolicy)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -5638,6 +5652,7 @@ cleanup: VIR_FREE(wrpolicy); VIR_FREE(usage); VIR_FREE(unit); + VIR_FREE(format); return def; @@ -13780,10 +13795,13 @@ virDomainFSDefFormat(virBufferPtr buf, if (def->fsdriver) { virBufferAsprintf(buf, " <driver type='%s'", fsdriver); + if (def->format) + virBufferAsprintf(buf, " format='%s'", + virStorageFileFormatTypeToString(def->format)); + /* Don't generate anything if wrpolicy is set to default */ - if (def->wrpolicy) { + if (def->wrpolicy) virBufferAsprintf(buf, " wrpolicy='%s'", wrpolicy); - } virBufferAddLit(buf, "/>\n"); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 21f7ce2..76a6dd0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -792,6 +792,7 @@ enum virDomainFSDriverType { VIR_DOMAIN_FS_DRIVER_TYPE_DEFAULT = 0, VIR_DOMAIN_FS_DRIVER_TYPE_PATH, VIR_DOMAIN_FS_DRIVER_TYPE_HANDLE, + VIR_DOMAIN_FS_DRIVER_TYPE_LOOP, VIR_DOMAIN_FS_DRIVER_TYPE_LAST }; @@ -818,9 +819,10 @@ enum virDomainFSWrpolicy { struct _virDomainFSDef { int type; - int fsdriver; - int accessmode; + int fsdriver; /* enum virDomainFSDriverType */ + int accessmode; /* enum virDomainFSAccessMode */ int wrpolicy; /* enum virDomainFSWrpolicy */ + int format; /* enum virStorageFileFormat */ unsigned long long usage; char *src; char *dst; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 144620c..24f2fab 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -138,7 +138,8 @@ VIR_ENUM_DECL(qemuDomainFSDriver) VIR_ENUM_IMPL(qemuDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, "local", "local", - "handle"); + "handle", + NULL); /** diff --git a/tests/lxcxml2xmldata/lxc-disk-formats.xml b/tests/lxcxml2xmldata/lxc-disk-formats.xml new file mode 100644 index 0000000..da53cf2 --- /dev/null +++ b/tests/lxcxml2xmldata/lxc-disk-formats.xml @@ -0,0 +1,26 @@ +<domain type='lxc'> + <name>demo</name> + <uuid>8369f1ac-7e46-e869-4ca5-759d51478066</uuid> + <memory unit='KiB'>500000</memory> + <currentMemory unit='KiB'>500000</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64'>exe</type> + <init>/bin/sh</init> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/libexec/libvirt_lxc</emulator> + <filesystem type='file' accessmode='passthrough'> + <driver type='loop' format='raw'/> + <source file='/root/container.img'/> + <target dir='/'/> + </filesystem> + <console type='pty'> + <target type='lxc' port='0'/> + </console> + </devices> +</domain> diff --git a/tests/lxcxml2xmltest.c b/tests/lxcxml2xmltest.c index 827c2fd..97f792c 100644 --- a/tests/lxcxml2xmltest.c +++ b/tests/lxcxml2xmltest.c @@ -128,6 +128,7 @@ mymain(void) DO_TEST("systemd"); DO_TEST("hostdev"); + DO_TEST("disk-formats"); virObjectUnref(caps); virObjectUnref(xmlopt); -- 1.8.2.1
On 03.05.2013 15:49, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Extend the <driver> element in filesystem devices to allow a storage format to be set. The new attribute uses 'format' to reflect the storage format. This is different from the <driver> element in disk devices which use 'type' to reflect the storage format. This is because the 'type' attribute on filesystem devices is already used for the driver backend, for which the disk devices use the 'name' attribute. Arggggh.
Anyway for disks we have
<driver name="qemu" type="raw"/>
And for filesystems this change means we now have
<driver type="loop" format="raw"/>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- docs/formatdomain.html.in | 24 ++++++++++ docs/schemas/domaincommon.rng | 73 ++++++++++++++++++++++--------- src/conf/domain_conf.c | 32 +++++++++++--- src/conf/domain_conf.h | 6 ++- src/qemu/qemu_command.c | 3 +- tests/lxcxml2xmldata/lxc-disk-formats.xml | 26 +++++++++++ tests/lxcxml2xmltest.c | 1 + 7 files changed, 134 insertions(+), 31 deletions(-) create mode 100644 tests/lxcxml2xmldata/lxc-disk-formats.xml
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 10596dc..7d1cfa6 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -888,7 +888,7 @@ <define name="diskspec"> <interleave> <optional> - <ref name="driver"/> + <ref name="diskDriver"/> </optional> <optional> <ref name='diskMirror'/> @@ -1270,7 +1270,7 @@ <!-- Disk may use a special driver for access. --> - <define name="driver"> + <define name="diskDriver"> <element name="driver"> <choice> <group> @@ -1314,13 +1314,13 @@ <optional> <attribute name='type'> <choice> - <ref name='diskFormat'/> + <ref name='storageFormat'/> <value>aio</value> <!-- back-compat for 'raw' --> </choice> </attribute> </optional> </define> - <define name='diskFormat'> + <define name='storageFormat'> <choice> <value>raw</value> <value>dir</value> @@ -1518,6 +1518,9 @@ <attribute name="type"> <value>file</value> </attribute> + <optional> + <ref name="diskDriver"/> + </optional> <interleave> <element name="source"> <attribute name="file"> @@ -1531,6 +1534,9 @@ <attribute name="type"> <value>block</value> </attribute> + <optional> + <ref name="diskDriver"/> + </optional> <interleave> <element name="source"> <attribute name="dev"> @@ -1547,6 +1553,9 @@ <value>mount</value> </attribute> </optional> + <optional> + <ref name="diskDriver"/> + </optional> <interleave> <element name="source"> <attribute name="dir"> @@ -1554,22 +1563,6 @@ </attribute> <empty/> </element> - <optional> - <element name="driver"> - <attribute name="type"> - <choice> - <value>path</value> - <value>handle</value> - </choice> - </attribute> - <optional> - <attribute name="wrpolicy"> - <value>immediate</value> - </attribute> - </optional> - <empty/> - </element> - </optional> </interleave> </group> <group>
After this chunk the 'syntax-check' complains. Could you fix that? Otherwise I have not spotted anything wrong. ACK. Michal
From: "Daniel P. Berrange" <berrange@redhat.com> The current code for setting up loop devices to LXC disks first does a switch() based on the disk format, then looks at the disk driver name. Reverse this so it first looks at the driver name, and then the disk format. This is more useful since the list of supported disk formats depends on what driver is used. The code for setting loop devices for LXC fs entries also needs to have the same logic added, now the XML schema supports this. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/lxc/lxc_controller.c | 76 +++++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 33 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index f1800eb..0f90e72 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -401,17 +401,31 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) if (fs->type != VIR_DOMAIN_FS_TYPE_FILE) continue; - fd = virLXCControllerSetupLoopDeviceFS(fs); - if (fd < 0) - goto cleanup; + if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_LOOP) { + if (fs->format != VIR_STORAGE_FILE_RAW && + fs->format != VIR_STORAGE_FILE_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("fs format %s is not supported"), + virStorageFileFormatTypeToString(fs->format)); + goto cleanup; + } - VIR_DEBUG("Saving loop fd %d", fd); - if (VIR_EXPAND_N(ctrl->loopDevFds, ctrl->nloopDevs, 1) < 0) { - VIR_FORCE_CLOSE(fd); - virReportOOMError(); - goto cleanup; + fd = virLXCControllerSetupLoopDeviceFS(fs); + if (fd < 0) + goto cleanup; + + VIR_DEBUG("Saving loop fd %d", fd); + if (VIR_EXPAND_N(ctrl->loopDevFds, ctrl->nloopDevs, 1) < 0) { + VIR_FORCE_CLOSE(fd); + virReportOOMError(); + goto cleanup; + } + ctrl->loopDevFds[ctrl->nloopDevs - 1] = fd; + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("fs driver %s is not supported"), + virDomainFSDriverTypeTypeToString(fs->fsdriver)); } - ctrl->loopDevFds[ctrl->nloopDevs - 1] = fd; } for (i = 0 ; i < ctrl->def->ndisks ; i++) { @@ -421,40 +435,36 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) if (disk->type != VIR_DOMAIN_DISK_TYPE_FILE) continue; - switch (disk->format) { - /* We treat 'none' as meaning 'raw' since we - * don't want to go into the auto-probing - * business for security reasons - */ - case VIR_STORAGE_FILE_RAW: - case VIR_STORAGE_FILE_NONE: - if (disk->driverName && - STRNEQ(disk->driverName, "loop")) { + if (!disk->driverName || + STREQ(disk->driverName, "loop")) { + if (disk->format != VIR_STORAGE_FILE_RAW && + disk->format != VIR_STORAGE_FILE_NONE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk driver %s is not supported"), - disk->driverName); + _("disk format %s is not supported"), + virStorageFileFormatTypeToString(disk->format)); goto cleanup; } + /* We treat 'none' as meaning 'raw' since we + * don't want to go into the auto-probing + * business for security reasons + */ fd = virLXCControllerSetupLoopDeviceDisk(disk); if (fd < 0) goto cleanup; - break; - default: + VIR_DEBUG("Saving loop fd %d", fd); + if (VIR_EXPAND_N(ctrl->loopDevFds, ctrl->nloopDevs, 1) < 0) { + VIR_FORCE_CLOSE(fd); + virReportOOMError(); + goto cleanup; + } + ctrl->loopDevFds[ctrl->nloopDevs - 1] = fd; + } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk format %s is not supported"), - virStorageFileFormatTypeToString(disk->format)); - goto cleanup; - } - - VIR_DEBUG("Saving loop fd %d", fd); - if (VIR_EXPAND_N(ctrl->loopDevFds, ctrl->nloopDevs, 1) < 0) { - VIR_FORCE_CLOSE(fd); - virReportOOMError(); - goto cleanup; + _("disk driver %s is not supported"), + disk->driverName); } - ctrl->loopDevFds[ctrl->nloopDevs - 1] = fd; } VIR_DEBUG("Setup all loop devices"); -- 1.8.2.1
On 03.05.2013 15:49, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The current code for setting up loop devices to LXC disks first does a switch() based on the disk format, then looks at the disk driver name. Reverse this so it first looks at the driver name, and then the disk format. This is more useful since the list of supported disk formats depends on what driver is used.
The code for setting loop devices for LXC fs entries also needs to have the same logic added, now the XML schema supports this.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/lxc/lxc_controller.c | 76 +++++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 33 deletions(-)
ACK Michal
From: "Daniel P. Berrange" <berrange@redhat.com> To correctly handle errors from readdir() you must set 'errno' to zero before invoking it & check its value afterwards to distinguish error from EOF. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/virfile.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/util/virfile.c b/src/util/virfile.c index 25a0501..9363468 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -531,6 +531,7 @@ static int virFileLoopDeviceOpen(char **dev_name) goto cleanup; } + errno = 0; while ((de = readdir(dh)) != NULL) { if (!STRPREFIX(de->d_name, "loop")) continue; @@ -562,10 +563,15 @@ static int virFileLoopDeviceOpen(char **dev_name) /* Oh well, try the next device */ VIR_FORCE_CLOSE(fd); VIR_FREE(looppath); + errno = 0; } - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to find a free loop device in /dev")); + if (errno != 0) + virReportSystemError(errno, "%s", + _("Unable to iterate over loop devices")); + else + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to find a free loop device in /dev")); cleanup: if (fd != -1) { -- 1.8.2.1
On 03.05.2013 15:49, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
To correctly handle errors from readdir() you must set 'errno' to zero before invoking it & check its value afterwards to distinguish error from EOF.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/virfile.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c index 25a0501..9363468 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -531,6 +531,7 @@ static int virFileLoopDeviceOpen(char **dev_name) goto cleanup; }
+ errno = 0; while ((de = readdir(dh)) != NULL) { if (!STRPREFIX(de->d_name, "loop")) continue; @@ -562,10 +563,15 @@ static int virFileLoopDeviceOpen(char **dev_name) /* Oh well, try the next device */ VIR_FORCE_CLOSE(fd); VIR_FREE(looppath); + errno = 0; }
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unable to find a free loop device in /dev")); + if (errno != 0) + virReportSystemError(errno, "%s", + _("Unable to iterate over loop devices")); + else + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to find a free loop device in /dev"));
cleanup: if (fd != -1) {
ACK Michal
From: "Daniel P. Berrange" <berrange@redhat.com> Add a virFileNBDDeviceAssociate method, which given a filename will setup a NBD device, using qemu-nbd as the server. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virfile.c | 143 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/virfile.h | 6 ++ 3 files changed, 150 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 98660dc..cea0f72 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1277,6 +1277,7 @@ virFileDirectFdFlag; virFileFclose; virFileFdopen; virFileLoopDeviceAssociate; +virFileNBDDeviceAssociate; virFileRewrite; virFileTouch; virFileUpdatePerm; diff --git a/src/util/virfile.c b/src/util/virfile.c index 9363468..4a5154e 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -638,6 +638,138 @@ cleanup: return lofd; } + +# define SYSFS_BLOCK_DIR "/sys/block" + + +static int +virFileNBDDeviceIsBusy(const char *devname) +{ + char *path; + int ret = -1; + + if (virAsprintf(&path, SYSFS_BLOCK_DIR "/%s/pid", + devname) < 0) { + virReportOOMError(); + return -1; + } + + if (access(path, F_OK) < 0) { + if (errno == ENOENT) + ret = 0; + else + virReportSystemError(errno, + _("Cannot check NBD device %s pid"), + devname); + goto cleanup; + } + ret = 1; + +cleanup: + VIR_FREE(path); + return ret; +} + + +static char * +virFileNBDDeviceFindUnused(void) +{ + DIR *dh; + char *ret = NULL; + struct dirent *de; + + if (!(dh = opendir(SYSFS_BLOCK_DIR))) { + virReportSystemError(errno, + _("Cannot read directory %s"), + SYSFS_BLOCK_DIR); + return NULL; + } + + errno = 0; + while ((de = readdir(dh)) != NULL) { + if (STRPREFIX(de->d_name, "nbd")) { + int rv = virFileNBDDeviceIsBusy(de->d_name); + if (rv < 0) + goto cleanup; + if (rv == 0) { + if (virAsprintf(&ret, "/dev/%s", de->d_name) < 0) { + virReportOOMError(); + goto cleanup; + } + goto cleanup; + } + } + errno = 0; + } + + if (errno != 0) + virReportSystemError(errno, "%s", + _("Unable to iterate over NBD devices")); + else + virReportSystemError(EBUSY, "%s", + _("No free NBD devices")); + +cleanup: + closedir(dh); + return ret; +} + + +int virFileNBDDeviceAssociate(const char *file, + enum virStorageFileFormat fmt, + bool readonly, + char **dev) +{ + char *nbddev; + char *qemunbd; + virCommandPtr cmd = NULL; + int ret = -1; + const char *fmtstr = NULL; + + if (!(nbddev = virFileNBDDeviceFindUnused())) + goto cleanup; + + if (!(qemunbd = virFindFileInPath("qemu-nbd"))) { + virReportSystemError(ENOENT, "%s", + _("Unable to find 'qemu-nbd' binary in $PATH")); + goto cleanup; + } + + if (fmt > 0) + fmtstr = virStorageFileFormatTypeToString(fmt); + + cmd = virCommandNew(qemunbd); + + /* Explicitly not trying to cope with old qemu-nbd which + * lacked --format. We want to see a fatal error in that + * case since it would be security flaw to continue */ + if (fmtstr) + virCommandAddArgList(cmd, "--format", fmtstr, NULL); + + if (readonly) + virCommandAddArg(cmd, "-r"); + + virCommandAddArgList(cmd, + "-n", /* Don't cache in qemu-nbd layer */ + "-c", nbddev, + file, NULL); + + /* qemu-nbd will daemonize itself */ + + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + *dev = nbddev; + nbddev = NULL; + ret = 0; + +cleanup: + VIR_FREE(nbddev); + VIR_FREE(qemunbd); + virCommandFree(cmd); + return ret; +} + #else /* __linux__ */ int virFileLoopDeviceAssociate(const char *file, @@ -650,6 +782,17 @@ int virFileLoopDeviceAssociate(const char *file, return -1; } +int virFileNBDDeviceAssociate(const char *file, + enum virStorageFileFormat fmt ATTRIBUTE_UNUSED, + bool readonly ATTRIBUTE_UNUSED, + char **dev ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, + _("Unable to associate file %s with NBD device"), + file); + return -1; +} + #endif /* __linux__ */ diff --git a/src/util/virfile.h b/src/util/virfile.h index 5f0dd2b..9bd8ea5 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -29,6 +29,7 @@ # include <stdio.h> # include "internal.h" +# include "virstoragefile.h" typedef enum virFileCloseFlags { VIR_FILE_CLOSE_PRESERVE_ERRNO = 1 << 0, @@ -108,6 +109,11 @@ int virFileUpdatePerm(const char *path, int virFileLoopDeviceAssociate(const char *file, char **dev); +int virFileNBDDeviceAssociate(const char *file, + enum virStorageFileFormat fmt, + bool readonly, + char **dev); + int virFileDeleteTree(const char *dir); #endif /* __VIR_FILES_H */ -- 1.8.2.1
On 03.05.2013 15:49, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Add a virFileNBDDeviceAssociate method, which given a filename will setup a NBD device, using qemu-nbd as the server.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virfile.c | 143 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/virfile.h | 6 ++ 3 files changed, 150 insertions(+)
ACK Michal
From: "Daniel P. Berrange" <berrange@redhat.com> The <filesystem> element can now accept a <driver type='nbd'/> as an alternative to 'loop'. The benefit of NBD is support for non-raw disk image formats. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- docs/formatdomain.html.in | 5 +++-- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 1 + tests/lxcxml2xmldata/lxc-disk-formats.xml | 5 +++++ 6 files changed, 13 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c95756e..768b54c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1985,8 +1985,9 @@ the <code>type</code> attribute selects the primary backend driver name, while the <code>format</code> attribute provides the format type. For example, LXC - supports a type of "loop", with a format of "raw". QEMU - supports a type of "path" or "handle", but no formats. + supports a type of "loop", with a format of "raw" or + "nbd" with any format. QEMU supports a type of "path" + or "handle", but no formats. </li> </ul> </dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7d1cfa6..73225fc 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1675,6 +1675,7 @@ <value>path</value> <value>handle</value> <value>loop</value> + <value>nbd</value> </choice> </attribute> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f858485..6bb30ae 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -343,7 +343,8 @@ VIR_ENUM_IMPL(virDomainFSDriverType, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, "default", "path", "handle", - "loop") + "loop", + "nbd") VIR_ENUM_IMPL(virDomainFSAccessMode, VIR_DOMAIN_FS_ACCESSMODE_LAST, "passthrough", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 76a6dd0..c9e5d66 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -793,6 +793,7 @@ enum virDomainFSDriverType { VIR_DOMAIN_FS_DRIVER_TYPE_PATH, VIR_DOMAIN_FS_DRIVER_TYPE_HANDLE, VIR_DOMAIN_FS_DRIVER_TYPE_LOOP, + VIR_DOMAIN_FS_DRIVER_TYPE_NBD, VIR_DOMAIN_FS_DRIVER_TYPE_LAST }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 24f2fab..894b1a9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -139,6 +139,7 @@ VIR_ENUM_IMPL(qemuDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, "local", "local", "handle", + NULL, NULL); diff --git a/tests/lxcxml2xmldata/lxc-disk-formats.xml b/tests/lxcxml2xmldata/lxc-disk-formats.xml index da53cf2..e953065 100644 --- a/tests/lxcxml2xmldata/lxc-disk-formats.xml +++ b/tests/lxcxml2xmldata/lxc-disk-formats.xml @@ -19,6 +19,11 @@ <source file='/root/container.img'/> <target dir='/'/> </filesystem> + <filesystem type='file' accessmode='passthrough'> + <driver type='nbd' format='qcow2'/> + <source file='/root/container.qcow2'/> + <target dir='/home'/> + </filesystem> <console type='pty'> <target type='lxc' port='0'/> </console> -- 1.8.2.1
On 03.05.2013 15:49, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The <filesystem> element can now accept a <driver type='nbd'/> as an alternative to 'loop'. The benefit of NBD is support for non-raw disk image formats.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- docs/formatdomain.html.in | 5 +++-- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 1 + tests/lxcxml2xmldata/lxc-disk-formats.xml | 5 +++++ 6 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c95756e..768b54c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1985,8 +1985,9 @@ the <code>type</code> attribute selects the primary backend driver name, while the <code>format</code> attribute provides the format type. For example, LXC - supports a type of "loop", with a format of "raw". QEMU - supports a type of "path" or "handle", but no formats. + supports a type of "loop", with a format of "raw" or + "nbd" with any format. QEMU supports a type of "path" + or "handle", but no formats. </li> </ul> </dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7d1cfa6..73225fc 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1675,6 +1675,7 @@ <value>path</value> <value>handle</value> <value>loop</value> + <value>nbd</value> </choice> </attribute> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f858485..6bb30ae 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -343,7 +343,8 @@ VIR_ENUM_IMPL(virDomainFSDriverType, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, "default", "path", "handle", - "loop") + "loop", + "nbd")
VIR_ENUM_IMPL(virDomainFSAccessMode, VIR_DOMAIN_FS_ACCESSMODE_LAST, "passthrough", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 76a6dd0..c9e5d66 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -793,6 +793,7 @@ enum virDomainFSDriverType { VIR_DOMAIN_FS_DRIVER_TYPE_PATH, VIR_DOMAIN_FS_DRIVER_TYPE_HANDLE, VIR_DOMAIN_FS_DRIVER_TYPE_LOOP, + VIR_DOMAIN_FS_DRIVER_TYPE_NBD,
VIR_DOMAIN_FS_DRIVER_TYPE_LAST }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 24f2fab..894b1a9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -139,6 +139,7 @@ VIR_ENUM_IMPL(qemuDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, "local", "local", "handle", + NULL, NULL);
diff --git a/tests/lxcxml2xmldata/lxc-disk-formats.xml b/tests/lxcxml2xmldata/lxc-disk-formats.xml index da53cf2..e953065 100644 --- a/tests/lxcxml2xmldata/lxc-disk-formats.xml +++ b/tests/lxcxml2xmldata/lxc-disk-formats.xml @@ -19,6 +19,11 @@ <source file='/root/container.img'/> <target dir='/'/> </filesystem> + <filesystem type='file' accessmode='passthrough'> + <driver type='nbd' format='qcow2'/> + <source file='/root/container.qcow2'/> + <target dir='/home'/> + </filesystem> <console type='pty'> <target type='lxc' port='0'/> </console>
ACK Michal
From: "Daniel P. Berrange" <berrange@redhat.com> The LXC driver can already configure <disk> or <filesystem> devices to use the loop device. This extends it to also allow for use of the NBD device, to support non-raw formats. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/lxc/lxc_controller.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 0f90e72..d92b722 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -389,6 +389,62 @@ static int virLXCControllerSetupLoopDeviceDisk(virDomainDiskDefPtr disk) } +static int virLXCControllerSetupNBDDeviceFS(virDomainFSDefPtr fs) +{ + char *dev; + + if (fs->format <= VIR_STORAGE_FILE_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("An explicit disk format must be specified")); + return -1; + } + + if (virFileNBDDeviceAssociate(fs->src, + fs->format, + fs->readonly, + &dev) < 0) + return -1; + + /* + * We now change it into a block device type, so that + * the rest of container setup 'just works' + */ + fs->type = VIR_DOMAIN_DISK_TYPE_BLOCK; + VIR_FREE(fs->src); + fs->src = dev; + + return 0; +} + + +static int virLXCControllerSetupNBDDeviceDisk(virDomainDiskDefPtr disk) +{ + char *dev; + + if (disk->format <= VIR_STORAGE_FILE_NONE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("An explicit disk format must be specified")); + return -1; + } + + if (virFileNBDDeviceAssociate(disk->src, + disk->format, + disk->readonly, + &dev) < 0) + return -1; + + /* + * We now change it into a block device type, so that + * the rest of container setup 'just works' + */ + disk->type = VIR_DOMAIN_DISK_TYPE_BLOCK; + VIR_FREE(disk->src); + disk->src = dev; + + return 0; +} + + static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) { size_t i; @@ -421,6 +477,9 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) goto cleanup; } ctrl->loopDevFds[ctrl->nloopDevs - 1] = fd; + } else if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_NBD) { + if (virLXCControllerSetupNBDDeviceFS(fs) < 0) + goto cleanup; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("fs driver %s is not supported"), @@ -435,8 +494,14 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) if (disk->type != VIR_DOMAIN_DISK_TYPE_FILE) continue; - if (!disk->driverName || - STREQ(disk->driverName, "loop")) { + /* If no driverName is set, we prefer 'loop' for + * dealing with raw or undefined formats, otherwise + * we use 'nbd'. + */ + if (STREQ_NULLABLE(disk->driverName, "loop") || + (!disk->driverName && + (disk->format == VIR_STORAGE_FILE_RAW || + disk->format == VIR_STORAGE_FILE_NONE))) { if (disk->format != VIR_STORAGE_FILE_RAW && disk->format != VIR_STORAGE_FILE_NONE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -460,6 +525,17 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) goto cleanup; } ctrl->loopDevFds[ctrl->nloopDevs - 1] = fd; + } else if (STREQ_NULLABLE(disk->driverName, "nbd") || + !disk->driverName) { + if (disk->cachemode != VIR_DOMAIN_DISK_CACHE_DEFAULT && + disk->cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Disk cache mode %s is not supported"), + virDomainDiskCacheTypeToString(disk->cachemode)); + goto cleanup; + } + if (virLXCControllerSetupNBDDeviceDisk(disk) < 0) + goto cleanup; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("disk driver %s is not supported"), -- 1.8.2.1
On 03.05.2013 15:49, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The LXC driver can already configure <disk> or <filesystem> devices to use the loop device. This extends it to also allow for use of the NBD device, to support non-raw formats.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/lxc/lxc_controller.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 2 deletions(-)
ACK Michal
On Fri, May 03, 2013 at 02:49:48PM +0100, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
A third version of:
https://www.redhat.com/archives/libvir-list/2013-April/msg01539.html
In v3
- Split patch 3 into 2 pieces - Added XML formatting tests - Fixed XML formatting - Remove !! from vars that are now bools
Ping. Not many changes to re-review against v2 so should be easy todo.... 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 :|
participants (2)
-
Daniel P. Berrange -
Michal Privoznik