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 | 25 ++++++++++++--- src/conf/domain_conf.h | 6 ++-- src/qemu/qemu_command.c | 3 +- 5 files changed, 102 insertions(+), 29 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 888c005..a707cc8 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.4</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 3976b82..206757f 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> @@ -1502,6 +1502,9 @@ <attribute name="type"> <value>file</value> </attribute> + <optional> + <ref name="diskDriver"/> + </optional> <interleave> <element name="source"> <attribute name="file"> @@ -1515,6 +1518,9 @@ <attribute name="type"> <value>block</value> </attribute> + <optional> + <ref name="diskDriver"/> + </optional> <interleave> <element name="source"> <attribute name="dev"> @@ -1531,6 +1537,9 @@ <value>mount</value> </attribute> </optional> + <optional> + <ref name="diskDriver"/> + </optional> <interleave> <element name="source"> <attribute name="dir"> @@ -1538,22 +1547,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> @@ -1562,6 +1555,9 @@ <value>bind</value> </attribute> </optional> + <optional> + <ref name="diskDriver"/> + </optional> <interleave> <element name="source"> <attribute name="dir"> @@ -1575,6 +1571,9 @@ <attribute name="type"> <value>template</value> </attribute> + <optional> + <ref name="diskDriver"/> + </optional> <interleave> <element name="source"> <attribute name="name"> @@ -1588,6 +1587,9 @@ <attribute name="type"> <value>ram</value> </attribute> + <optional> + <ref name="diskDriver"/> + </optional> <interleave> <element name="source"> <attribute name="usage"> @@ -1645,6 +1647,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 @@ -3754,7 +3785,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 dc0ecaa..582308b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -336,7 +336,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", @@ -5368,6 +5369,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; @@ -5436,9 +5438,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; @@ -5446,12 +5452,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, @@ -5511,6 +5525,7 @@ cleanup: VIR_FREE(wrpolicy); VIR_FREE(usage); VIR_FREE(unit); + VIR_FREE(format); return def; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f1f01fa..a65aabc 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -767,6 +767,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 }; @@ -793,9 +794,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 05c12b2..05c2a41 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -130,7 +130,8 @@ VIR_ENUM_DECL(qemuDomainFSDriver) VIR_ENUM_IMPL(qemuDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, "local", "local", - "handle"); + "handle", + NULL); /** -- 1.7.11.7

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 740b872..a91ea79 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.7.11.7

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 | 151 ++++++++++++++++++++++++++++++++++++++++++++++- src/util/virfile.h | 6 ++ 3 files changed, 155 insertions(+), 3 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f778e9c..a857a0b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1272,6 +1272,7 @@ virFileDirectFdFlag; virFileFclose; virFileFdopen; virFileLoopDeviceAssociate; +virFileNBDDeviceAssociate; virFileRewrite; virFileTouch; virFileUpdatePerm; diff --git a/src/util/virfile.c b/src/util/virfile.c index fbaeedd..034648b 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -530,6 +530,7 @@ static int virFileLoopDeviceOpen(char **dev_name) goto cleanup; } + errno = 0; while ((de = readdir(dh)) != NULL) { if (!STRPREFIX(de->d_name, "loop")) continue; @@ -561,10 +562,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) { @@ -631,10 +637,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; + } + + 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; + } + } + } + + 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) { + virCommandAddArg(cmd, "--format"); + virCommandAddArg(cmd, fmtstr); + } + + 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, - char **dev ATTRIBUTE_UNUSED) + char **dev ATTRIBUTE) { virReportSystemError(ENOSYS, _("Unable to associate file %s with loop device"), @@ -643,6 +777,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.7.11.7