[libvirt] [PATCH 0/7] Add support for qcow2 backing files to sVirt

Hey, https://bugzilla.redhat.com/497131 shows that sVirt cannot currently handle qcow2 files with a backing store. Because we do not re-label the backing store, qemu cannot read it. The fix is fairly straightforward - when we go to re-label a file, check its header and if it has a backing store, re-label that too. Most of this series involves moving the header probing code from the storage driver into libvirt_util so that the security driver can use it. Cheers, Mark.

Seems standard to include internal.h in order to pull in libvirt.h * src/util/util.h: include internal.h --- src/util/util.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/util/util.h b/src/util/util.h index dbfdc45..77f50ed 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -26,6 +26,7 @@ #define __VIR_UTIL_H__ #include "verify.h" +#include "internal.h" #include <sys/select.h> #include <sys/types.h> -- 1.6.2.5

On Fri, Sep 25, 2009 at 02:27:27PM +0100, Mark McLoughlin wrote:
Seems standard to include internal.h in order to pull in libvirt.h
* src/util/util.h: include internal.h --- src/util/util.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
There don't appear to be any compile issues without this patch -it'll be pulled in already via one of the many other headers it includes Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

This is so we can use it in the storage file probing code which is also moving to src/util The real code change is renaming virStorageReportError() to virStorageEncryptionReportError() * src/util/storage_encryption.[ch]: add * src/conf/storage_encryption_conf.[ch]: remove * src/conf/domain_conf.h, storage/conf/storage_conf.h: include storage_encryption.h * src/Makefile.am: build src/util/storage_encryption.[ch]; note libvirt_lxc now needs to be linked with $(SELINUX_LIBS) * proxy/Makefile.am: ditto * po/POTFILES.in, src/libvirt_private.syms: update --- po/POTFILES.in | 2 +- proxy/Makefile.am | 2 +- src/Makefile.am | 52 +++--- src/conf/domain_conf.h | 2 +- src/conf/storage_conf.h | 2 +- src/conf/storage_encryption_conf.c | 294 ----------------------------------- src/conf/storage_encryption_conf.h | 80 ---------- src/libvirt_private.syms | 2 +- src/util/storage_encryption.c | 301 ++++++++++++++++++++++++++++++++++++ src/util/storage_encryption.h | 80 ++++++++++ 10 files changed, 412 insertions(+), 405 deletions(-) delete mode 100644 src/conf/storage_encryption_conf.c delete mode 100644 src/conf/storage_encryption_conf.h create mode 100644 src/util/storage_encryption.c create mode 100644 src/util/storage_encryption.h diff --git a/po/POTFILES.in b/po/POTFILES.in index 67769c8..9f21459 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -7,7 +7,6 @@ src/conf/network_conf.c src/conf/node_device_conf.c src/conf/secret_conf.c src/conf/storage_conf.c -src/conf/storage_encryption_conf.c src/datatypes.c src/interface/netcf_driver.c src/libvirt.c @@ -45,6 +44,7 @@ src/util/conf.c src/util/iptables.c src/util/logging.c src/util/pci.c +src/util/storage_encryption.c src/util/util.c src/util/uuid.c src/util/virterror.c diff --git a/proxy/Makefile.am b/proxy/Makefile.am index 3e0050b..2ae6ef9 100644 --- a/proxy/Makefile.am +++ b/proxy/Makefile.am @@ -17,12 +17,12 @@ libvirt_proxy_SOURCES = libvirt_proxy.c \ @top_srcdir@/src/util/buf.c \ @top_srcdir@/src/util/logging.c \ @top_srcdir@/src/util/memory.c \ + @top_srcdir@/src/util/storage_encryption.c \ @top_srcdir@/src/util/threads.c \ @top_srcdir@/src/util/util.c \ @top_srcdir@/src/util/uuid.c \ @top_srcdir@/src/util/virterror.c \ @top_srcdir@/src/conf/capabilities.c \ - @top_srcdir@/src/conf/storage_encryption_conf.c \ @top_srcdir@/src/conf/domain_conf.c \ @top_srcdir@/src/xen/xend_internal.c \ @top_srcdir@/src/xen/xen_hypervisor.c \ diff --git a/src/Makefile.am b/src/Makefile.am index 9cbec47..5bddb58 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -35,26 +35,27 @@ mod_LTLIBRARIES = # These files are not related to driver APIs. Simply generic # helper APIs for various purposes -UTIL_SOURCES = \ - util/bridge.c util/bridge.h \ - util/buf.c util/buf.h \ - util/conf.c util/conf.h \ - util/cgroup.c util/cgroup.h \ - util/event.c util/event.h \ - util/hash.c util/hash.h \ - util/iptables.c util/iptables.h \ - util/logging.c util/logging.h \ - util/memory.c util/memory.h \ - util/pci.c util/pci.h \ - util/hostusb.c util/hostusb.h \ - util/qparams.c util/qparams.h \ - util/stats_linux.c util/stats_linux.h \ - util/threads.c util/threads.h \ - util/threads-pthread.h \ - util/threads-win32.h \ - util/uuid.c util/uuid.h \ - util/util.c util/util.h \ - util/xml.c util/xml.h \ +UTIL_SOURCES = \ + util/bridge.c util/bridge.h \ + util/buf.c util/buf.h \ + util/conf.c util/conf.h \ + util/cgroup.c util/cgroup.h \ + util/event.c util/event.h \ + util/hash.c util/hash.h \ + util/iptables.c util/iptables.h \ + util/logging.c util/logging.h \ + util/memory.c util/memory.h \ + util/pci.c util/pci.h \ + util/hostusb.c util/hostusb.h \ + util/qparams.c util/qparams.h \ + util/stats_linux.c util/stats_linux.h \ + util/storage_encryption.c util/storage_encryption.h \ + util/threads.c util/threads.h \ + util/threads-pthread.h \ + util/threads-win32.h \ + util/uuid.c util/uuid.h \ + util/util.c util/util.h \ + util/xml.c util/xml.h \ util/virterror.c util/virterror_internal.h EXTRA_DIST += util/threads-pthread.c util/threads-win32.c @@ -98,16 +99,12 @@ SECRET_CONF_SOURCES = \ NODE_DEVICE_CONF_SOURCES = \ conf/node_device_conf.c conf/node_device_conf.h -ENCRYPTION_CONF_SOURCES = \ - conf/storage_encryption_conf.c conf/storage_encryption_conf.h - CONF_SOURCES = \ $(DOMAIN_CONF_SOURCES) \ $(DOMAIN_EVENT_SOURCES) \ $(NETWORK_CONF_SOURCES) \ $(NODE_DEVICE_CONF_SOURCES) \ $(STORAGE_CONF_SOURCES) \ - $(ENCRYPTION_CONF_SOURCES) \ $(INTERFACE_CONF_SOURCES) \ $(SECRET_CONF_SOURCES) @@ -779,10 +776,13 @@ libvirt_lxc_SOURCES = \ $(LXC_CONTROLLER_SOURCES) \ $(UTIL_SOURCES) \ $(NODE_INFO_SOURCES) \ - $(ENCRYPTION_CONF_SOURCES) \ $(DOMAIN_CONF_SOURCES) libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDCFLAGS) $(CAPNG_LIBS) -libvirt_lxc_LDADD = $(LIBXML_LIBS) $(NUMACTL_LIBS) ../gnulib/lib/libgnu.la +libvirt_lxc_LDADD = \ + $(LIBXML_LIBS) \ + $(NUMACTL_LIBS) \ + $(SELINUX_LIBS) \ + ../gnulib/lib/libgnu.la libvirt_lxc_CFLAGS = \ $(LIBPARTED_CFLAGS) \ $(NUMACTL_CFLAGS) \ diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7c918a7..1db5b2b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -30,7 +30,7 @@ #include "internal.h" #include "capabilities.h" -#include "storage_encryption_conf.h" +#include "storage_encryption.h" #include "util.h" #include "threads.h" diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 421d305..00dd102 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -26,7 +26,7 @@ #include "internal.h" #include "util.h" -#include "storage_encryption_conf.h" +#include "storage_encryption.h" #include "threads.h" #include <libxml/tree.h> diff --git a/src/conf/storage_encryption_conf.c b/src/conf/storage_encryption_conf.c deleted file mode 100644 index b97b989..0000000 --- a/src/conf/storage_encryption_conf.c +++ /dev/null @@ -1,294 +0,0 @@ -/* - * storage_encryption_conf.c: volume encryption information - * - * Copyright (C) 2009 Red Hat, Inc. - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA - * - * Red Hat Author: Miloslav Trmač <mitr@redhat.com> - */ - -#include <config.h> - -#include <fcntl.h> -#include <unistd.h> - -#include "internal.h" - -#include "buf.h" -#include "memory.h" -#include "storage_conf.h" -#include "storage_encryption_conf.h" -#include "util.h" -#include "xml.h" -#include "virterror_internal.h" -#include "uuid.h" - -#define VIR_FROM_THIS VIR_FROM_STORAGE - -VIR_ENUM_IMPL(virStorageEncryptionSecretType, - VIR_STORAGE_ENCRYPTION_SECRET_TYPE_LAST, "passphrase") - -VIR_ENUM_IMPL(virStorageEncryptionFormat, - VIR_STORAGE_ENCRYPTION_FORMAT_LAST, - "default", "qcow") - -static void -virStorageEncryptionSecretFree(virStorageEncryptionSecretPtr secret) -{ - if (!secret) - return; - VIR_FREE(secret); -} - -void -virStorageEncryptionFree(virStorageEncryptionPtr enc) -{ - size_t i; - - if (!enc) - return; - - for (i = 0; i < enc->nsecrets; i++) - virStorageEncryptionSecretFree(enc->secrets[i]); - VIR_FREE(enc->secrets); - VIR_FREE(enc); -} - -#ifndef PROXY - -static virStorageEncryptionSecretPtr -virStorageEncryptionSecretParse(virConnectPtr conn, xmlXPathContextPtr ctxt, - xmlNodePtr node) -{ - xmlNodePtr old_node; - virStorageEncryptionSecretPtr ret; - char *type_str; - int type; - char *uuidstr = NULL; - - if (VIR_ALLOC(ret) < 0) { - virReportOOMError(conn); - return NULL; - } - - old_node = ctxt->node; - ctxt->node = node; - - type_str = virXPathString(conn, "string(./@type)", ctxt); - if (type_str == NULL) { - virStorageReportError(conn, VIR_ERR_XML_ERROR, "%s", - _("unknown volume encryption secret type")); - goto cleanup; - } - type = virStorageEncryptionSecretTypeTypeFromString(type_str); - if (type < 0) { - virStorageReportError(conn, VIR_ERR_XML_ERROR, - _("unknown volume encryption secret type %s"), - type_str); - VIR_FREE(type_str); - goto cleanup; - } - VIR_FREE(type_str); - ret->type = type; - - uuidstr = virXPathString(conn, "string(./@uuid)", ctxt); - if (uuidstr) { - if (virUUIDParse(uuidstr, ret->uuid) < 0) { - virStorageReportError(conn, VIR_ERR_XML_ERROR, - _("malformed volume encryption uuid '%s'"), - uuidstr); - goto cleanup; - } - } else { - virStorageReportError(conn, VIR_ERR_XML_ERROR, "%s", - _("missing volume encryption uuid")); - goto cleanup; - } - ctxt->node = old_node; - return ret; - - cleanup: - virStorageEncryptionSecretFree(ret); - VIR_FREE(uuidstr); - ctxt->node = old_node; - return NULL; -} - -static virStorageEncryptionPtr -virStorageEncryptionParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt) -{ - xmlNodePtr *nodes = NULL; - virStorageEncryptionPtr ret; - char *format_str; - int format, i, n; - - if (VIR_ALLOC(ret) < 0) { - virReportOOMError(conn); - return NULL; - } - - format_str = virXPathString(conn, "string(./@format)", ctxt); - if (format_str == NULL) { - virStorageReportError(conn, VIR_ERR_XML_ERROR, "%s", - _("unknown volume encryption format")); - goto cleanup; - } - format = virStorageEncryptionFormatTypeFromString(format_str); - if (format < 0) { - virStorageReportError(conn, VIR_ERR_XML_ERROR, - _("unknown volume encryption format type %s"), - format_str); - VIR_FREE(format_str); - goto cleanup; - } - VIR_FREE(format_str); - ret->format = format; - - n = virXPathNodeSet(conn, "./secret", ctxt, &nodes); - if (n < 0){ - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot extract volume encryption secrets")); - goto cleanup; - } - if (n != 0 && VIR_ALLOC_N(ret->secrets, n) < 0) { - virReportOOMError(conn); - goto cleanup; - } - ret->nsecrets = n; - for (i = 0; i < n; i++) { - ret->secrets[i] = virStorageEncryptionSecretParse(conn, ctxt, nodes[i]); - if (ret->secrets[i] == NULL) - goto cleanup; - } - VIR_FREE(nodes); - - return ret; - - cleanup: - VIR_FREE(nodes); - virStorageEncryptionFree(ret); - return NULL; -} - -virStorageEncryptionPtr -virStorageEncryptionParseNode(virConnectPtr conn, - xmlDocPtr xml, xmlNodePtr root) -{ - xmlXPathContextPtr ctxt = NULL; - virStorageEncryptionPtr enc = NULL; - - if (STRNEQ((const char *) root->name, "encryption")) { - virStorageReportError(conn, VIR_ERR_XML_ERROR, - "%s", _("unknown root element for volume " - "encryption information")); - goto cleanup; - } - - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - virReportOOMError(conn); - goto cleanup; - } - - ctxt->node = root; - enc = virStorageEncryptionParseXML(conn, ctxt); - - cleanup: - xmlXPathFreeContext(ctxt); - return enc; -} -#endif /* ! PROXY */ - - -static int -virStorageEncryptionSecretFormat(virConnectPtr conn, - virBufferPtr buf, - virStorageEncryptionSecretPtr secret) -{ - const char *type; - char uuidstr[VIR_UUID_STRING_BUFLEN]; - - type = virStorageEncryptionSecretTypeTypeToString(secret->type); - if (!type) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", - _("unexpected volume encryption secret type")); - return -1; - } - - virUUIDFormat(secret->uuid, uuidstr); - virBufferVSprintf(buf, " <secret type='%s' uuid='%s'/>\n", type, uuidstr); - return 0; -} - -int -virStorageEncryptionFormat(virConnectPtr conn, - virBufferPtr buf, - virStorageEncryptionPtr enc) -{ - const char *format; - size_t i; - - format = virStorageEncryptionFormatTypeToString(enc->format); - if (!format) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, - "%s", _("unexpected encryption format")); - return -1; - } - virBufferVSprintf(buf, " <encryption format='%s'>\n", format); - - for (i = 0; i < enc->nsecrets; i++) { - if (virStorageEncryptionSecretFormat(conn, buf, enc->secrets[i]) < 0) - return -1; - } - - virBufferAddLit(buf, " </encryption>\n"); - - return 0; -} - -int -virStorageGenerateQcowPassphrase(virConnectPtr conn, unsigned char *dest) -{ - int fd; - size_t i; - - /* A qcow passphrase is up to 16 bytes, with any data following a NUL - ignored. Prohibit control and non-ASCII characters to avoid possible - unpleasant surprises with the qemu monitor input mechanism. */ - fd = open("/dev/urandom", O_RDONLY); - if (fd < 0) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot open /dev/urandom")); - return -1; - } - i = 0; - while (i < VIR_STORAGE_QCOW_PASSPHRASE_SIZE) { - ssize_t r; - - while ((r = read(fd, dest + i, 1)) == -1 && errno == EINTR) - ; - if (r <= 0) { - virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot read from /dev/urandom")); - close(fd); - return -1; - } - if (dest[i] >= 0x20 && dest[i] <= 0x7E) - i++; /* Got an acceptable character */ - } - close(fd); - return 0; -} diff --git a/src/conf/storage_encryption_conf.h b/src/conf/storage_encryption_conf.h deleted file mode 100644 index 5d0bc3c..0000000 --- a/src/conf/storage_encryption_conf.h +++ /dev/null @@ -1,80 +0,0 @@ -/* - * storage_encryption_conf.h: volume encryption information - * - * Copyright (C) 2009 Red Hat, Inc. - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA - * - * Red Hat Author: Miloslav Trmač <mitr@redhat.com> - */ - -#ifndef __VIR_STORAGE_ENCRYPTION_H__ -#define __VIR_STORAGE_ENCRYPTION_H__ - -#include "internal.h" -#include "buf.h" -#include "util.h" - -#include <stdbool.h> -#include <libxml/tree.h> - -enum virStorageEncryptionSecretType { - VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE = 0, - - VIR_STORAGE_ENCRYPTION_SECRET_TYPE_LAST -}; -VIR_ENUM_DECL(virStorageEncryptionSecretType) - -typedef struct _virStorageEncryptionSecret virStorageEncryptionSecret; -typedef virStorageEncryptionSecret *virStorageEncryptionSecretPtr; -struct _virStorageEncryptionSecret { - int type; /* enum virStorageEncryptionSecretType */ - unsigned char uuid[VIR_UUID_BUFLEN]; -}; - -enum virStorageEncryptionFormat { - /* "default" is only valid for volume creation */ - VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT = 0, - VIR_STORAGE_ENCRYPTION_FORMAT_QCOW, /* Both qcow and qcow2 */ - - VIR_STORAGE_ENCRYPTION_FORMAT_LAST, -}; -VIR_ENUM_DECL(virStorageEncryptionFormat) - -typedef struct _virStorageEncryption virStorageEncryption; -typedef virStorageEncryption *virStorageEncryptionPtr; -struct _virStorageEncryption { - int format; /* enum virStorageEncryptionFormat */ - - size_t nsecrets; - virStorageEncryptionSecretPtr *secrets; -}; - -void virStorageEncryptionFree(virStorageEncryptionPtr enc); - -virStorageEncryptionPtr virStorageEncryptionParseNode(virConnectPtr conn, - xmlDocPtr xml, - xmlNodePtr root); -int virStorageEncryptionFormat(virConnectPtr conn, virBufferPtr buf, - virStorageEncryptionPtr enc); - -/* A helper for VIR_STORAGE_ENCRYPTION_FORMAT_QCOW */ -enum { - VIR_STORAGE_QCOW_PASSPHRASE_SIZE = 16 -}; - -int virStorageGenerateQcowPassphrase(virConnectPtr conn, unsigned char *dest); - -#endif /* __VIR_STORAGE_ENCRYPTION_H__ */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a6668f3..e42a5e4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -385,7 +385,7 @@ virStoragePartedFsTypeTypeToString; virStoragePoolObjLock; virStoragePoolObjUnlock; -# storage_encryption_conf.h +# storage_encryption.h virStorageEncryptionFree; virStorageEncryptionDropSecrets; virStorageEncryptionParseNode; diff --git a/src/util/storage_encryption.c b/src/util/storage_encryption.c new file mode 100644 index 0000000..f9bef2d --- /dev/null +++ b/src/util/storage_encryption.c @@ -0,0 +1,301 @@ +/* + * storage_encryption.c: volume encryption information + * + * Copyright (C) 2009 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Red Hat Author: Miloslav Trmač <mitr@redhat.com> + */ + +#include <config.h> + +#include "storage_encryption.h" + +#include <fcntl.h> +#include <unistd.h> + +#include "internal.h" +#include "buf.h" +#include "memory.h" +#include "util.h" +#include "xml.h" +#include "virterror_internal.h" +#include "uuid.h" + +#define VIR_FROM_THIS VIR_FROM_STORAGE + +#define virStorageEncryptionReportError(conn, code, fmt...) \ + virReportErrorHelper(conn, VIR_FROM_STORAGE, code, __FILE__, \ + __FUNCTION__, __LINE__, fmt) + + +VIR_ENUM_IMPL(virStorageEncryptionSecretType, + VIR_STORAGE_ENCRYPTION_SECRET_TYPE_LAST, "passphrase") + +VIR_ENUM_IMPL(virStorageEncryptionFormat, + VIR_STORAGE_ENCRYPTION_FORMAT_LAST, + "default", "qcow") + +static void +virStorageEncryptionSecretFree(virStorageEncryptionSecretPtr secret) +{ + if (!secret) + return; + VIR_FREE(secret); +} + +void +virStorageEncryptionFree(virStorageEncryptionPtr enc) +{ + size_t i; + + if (!enc) + return; + + for (i = 0; i < enc->nsecrets; i++) + virStorageEncryptionSecretFree(enc->secrets[i]); + VIR_FREE(enc->secrets); + VIR_FREE(enc); +} + +#ifndef PROXY + +static virStorageEncryptionSecretPtr +virStorageEncryptionSecretParse(virConnectPtr conn, xmlXPathContextPtr ctxt, + xmlNodePtr node) +{ + xmlNodePtr old_node; + virStorageEncryptionSecretPtr ret; + char *type_str; + int type; + char *uuidstr = NULL; + + if (VIR_ALLOC(ret) < 0) { + virReportOOMError(conn); + return NULL; + } + + old_node = ctxt->node; + ctxt->node = node; + + type_str = virXPathString(conn, "string(./@type)", ctxt); + if (type_str == NULL) { + virStorageEncryptionReportError(conn, VIR_ERR_XML_ERROR, "%s", + _("unknown volume encryption " + "secret type")); + goto cleanup; + } + type = virStorageEncryptionSecretTypeTypeFromString(type_str); + if (type < 0) { + virStorageEncryptionReportError(conn, VIR_ERR_XML_ERROR, + _("unknown volume encryption " + "secret type %s"), type_str); + VIR_FREE(type_str); + goto cleanup; + } + VIR_FREE(type_str); + ret->type = type; + + uuidstr = virXPathString(conn, "string(./@uuid)", ctxt); + if (uuidstr) { + if (virUUIDParse(uuidstr, ret->uuid) < 0) { + virStorageEncryptionReportError(conn, VIR_ERR_XML_ERROR, + _("malformed volume encryption " + "uuid '%s'"), uuidstr); + goto cleanup; + } + } else { + virStorageEncryptionReportError(conn, VIR_ERR_XML_ERROR, "%s", + _("missing volume encryption uuid")); + goto cleanup; + } + ctxt->node = old_node; + return ret; + + cleanup: + virStorageEncryptionSecretFree(ret); + VIR_FREE(uuidstr); + ctxt->node = old_node; + return NULL; +} + +static virStorageEncryptionPtr +virStorageEncryptionParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt) +{ + xmlNodePtr *nodes = NULL; + virStorageEncryptionPtr ret; + char *format_str; + int format, i, n; + + if (VIR_ALLOC(ret) < 0) { + virReportOOMError(conn); + return NULL; + } + + format_str = virXPathString(conn, "string(./@format)", ctxt); + if (format_str == NULL) { + virStorageEncryptionReportError(conn, VIR_ERR_XML_ERROR, "%s", + _("unknown volume encryption format")); + goto cleanup; + } + format = virStorageEncryptionFormatTypeFromString(format_str); + if (format < 0) { + virStorageEncryptionReportError(conn, VIR_ERR_XML_ERROR, + _("unknown volume encryption format " + "type %s"), format_str); + VIR_FREE(format_str); + goto cleanup; + } + VIR_FREE(format_str); + ret->format = format; + + n = virXPathNodeSet(conn, "./secret", ctxt, &nodes); + if (n < 0){ + virStorageEncryptionReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot extract volume encryption " + "secrets")); + goto cleanup; + } + if (n != 0 && VIR_ALLOC_N(ret->secrets, n) < 0) { + virReportOOMError(conn); + goto cleanup; + } + ret->nsecrets = n; + for (i = 0; i < n; i++) { + ret->secrets[i] = virStorageEncryptionSecretParse(conn, ctxt, nodes[i]); + if (ret->secrets[i] == NULL) + goto cleanup; + } + VIR_FREE(nodes); + + return ret; + + cleanup: + VIR_FREE(nodes); + virStorageEncryptionFree(ret); + return NULL; +} + +virStorageEncryptionPtr +virStorageEncryptionParseNode(virConnectPtr conn, + xmlDocPtr xml, xmlNodePtr root) +{ + xmlXPathContextPtr ctxt = NULL; + virStorageEncryptionPtr enc = NULL; + + if (STRNEQ((const char *) root->name, "encryption")) { + virStorageEncryptionReportError(conn, VIR_ERR_XML_ERROR, "%s", + _("unknown root element for volume " + "encryption information")); + goto cleanup; + } + + ctxt = xmlXPathNewContext(xml); + if (ctxt == NULL) { + virReportOOMError(conn); + goto cleanup; + } + + ctxt->node = root; + enc = virStorageEncryptionParseXML(conn, ctxt); + + cleanup: + xmlXPathFreeContext(ctxt); + return enc; +} +#endif /* ! PROXY */ + + +static int +virStorageEncryptionSecretFormat(virConnectPtr conn, + virBufferPtr buf, + virStorageEncryptionSecretPtr secret) +{ + const char *type; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + type = virStorageEncryptionSecretTypeTypeToString(secret->type); + if (!type) { + virStorageEncryptionReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("unexpected volume encryption " + "secret type")); + return -1; + } + + virUUIDFormat(secret->uuid, uuidstr); + virBufferVSprintf(buf, " <secret type='%s' uuid='%s'/>\n", type, uuidstr); + return 0; +} + +int +virStorageEncryptionFormat(virConnectPtr conn, + virBufferPtr buf, + virStorageEncryptionPtr enc) +{ + const char *format; + size_t i; + + format = virStorageEncryptionFormatTypeToString(enc->format); + if (!format) { + virStorageEncryptionReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("unexpected encryption format")); + return -1; + } + virBufferVSprintf(buf, " <encryption format='%s'>\n", format); + + for (i = 0; i < enc->nsecrets; i++) { + if (virStorageEncryptionSecretFormat(conn, buf, enc->secrets[i]) < 0) + return -1; + } + + virBufferAddLit(buf, " </encryption>\n"); + + return 0; +} + +int +virStorageGenerateQcowPassphrase(virConnectPtr conn, unsigned char *dest) +{ + int fd; + size_t i; + + /* A qcow passphrase is up to 16 bytes, with any data following a NUL + ignored. Prohibit control and non-ASCII characters to avoid possible + unpleasant surprises with the qemu monitor input mechanism. */ + fd = open("/dev/urandom", O_RDONLY); + if (fd < 0) { + virStorageEncryptionReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot open /dev/urandom")); + return -1; + } + i = 0; + while (i < VIR_STORAGE_QCOW_PASSPHRASE_SIZE) { + ssize_t r; + + while ((r = read(fd, dest + i, 1)) == -1 && errno == EINTR) + ; + if (r <= 0) { + virStorageEncryptionReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot read from /dev/urandom")); + close(fd); + return -1; + } + if (dest[i] >= 0x20 && dest[i] <= 0x7E) + i++; /* Got an acceptable character */ + } + close(fd); + return 0; +} diff --git a/src/util/storage_encryption.h b/src/util/storage_encryption.h new file mode 100644 index 0000000..a6fe0f7 --- /dev/null +++ b/src/util/storage_encryption.h @@ -0,0 +1,80 @@ +/* + * storage_encryption.h: volume encryption information + * + * Copyright (C) 2009 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Red Hat Author: Miloslav Trmač <mitr@redhat.com> + */ + +#ifndef __VIR_STORAGE_ENCRYPTION_H__ +#define __VIR_STORAGE_ENCRYPTION_H__ + +#include "internal.h" +#include "buf.h" +#include "util.h" + +#include <stdbool.h> +#include <libxml/tree.h> + +enum virStorageEncryptionSecretType { + VIR_STORAGE_ENCRYPTION_SECRET_TYPE_PASSPHRASE = 0, + + VIR_STORAGE_ENCRYPTION_SECRET_TYPE_LAST +}; +VIR_ENUM_DECL(virStorageEncryptionSecretType) + +typedef struct _virStorageEncryptionSecret virStorageEncryptionSecret; +typedef virStorageEncryptionSecret *virStorageEncryptionSecretPtr; +struct _virStorageEncryptionSecret { + int type; /* enum virStorageEncryptionSecretType */ + unsigned char uuid[VIR_UUID_BUFLEN]; +}; + +enum virStorageEncryptionFormat { + /* "default" is only valid for volume creation */ + VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT = 0, + VIR_STORAGE_ENCRYPTION_FORMAT_QCOW, /* Both qcow and qcow2 */ + + VIR_STORAGE_ENCRYPTION_FORMAT_LAST, +}; +VIR_ENUM_DECL(virStorageEncryptionFormat) + +typedef struct _virStorageEncryption virStorageEncryption; +typedef virStorageEncryption *virStorageEncryptionPtr; +struct _virStorageEncryption { + int format; /* enum virStorageEncryptionFormat */ + + size_t nsecrets; + virStorageEncryptionSecretPtr *secrets; +}; + +void virStorageEncryptionFree(virStorageEncryptionPtr enc); + +virStorageEncryptionPtr virStorageEncryptionParseNode(virConnectPtr conn, + xmlDocPtr xml, + xmlNodePtr root); +int virStorageEncryptionFormat(virConnectPtr conn, virBufferPtr buf, + virStorageEncryptionPtr enc); + +/* A helper for VIR_STORAGE_ENCRYPTION_FORMAT_QCOW */ +enum { + VIR_STORAGE_QCOW_PASSPHRASE_SIZE = 16 +}; + +int virStorageGenerateQcowPassphrase(virConnectPtr conn, unsigned char *dest); + +#endif /* __VIR_STORAGE_ENCRYPTION_H__ */ -- 1.6.2.5

On Fri, Sep 25, 2009 at 02:27:28PM +0100, Mark McLoughlin wrote:
This is so we can use it in the storage file probing code which is also moving to src/util
The real code change is renaming virStorageReportError() to virStorageEncryptionReportError()
* src/util/storage_encryption.[ch]: add
* src/conf/storage_encryption_conf.[ch]: remove
* src/conf/domain_conf.h, storage/conf/storage_conf.h: include storage_encryption.h
* src/Makefile.am: build src/util/storage_encryption.[ch]; note libvirt_lxc now needs to be linked with $(SELINUX_LIBS)
* proxy/Makefile.am: ditto
* po/POTFILES.in, src/libvirt_private.syms: update --- po/POTFILES.in | 2 +- proxy/Makefile.am | 2 +- src/Makefile.am | 52 +++--- src/conf/domain_conf.h | 2 +- src/conf/storage_conf.h | 2 +- src/conf/storage_encryption_conf.c | 294 ----------------------------------- src/conf/storage_encryption_conf.h | 80 ---------- src/libvirt_private.syms | 2 +- src/util/storage_encryption.c | 301 ++++++++++++++++++++++++++++++++++++ src/util/storage_encryption.h | 80 ++++++++++
NACK, this is really not very desirable - it was intended that there would be a strict separation between the XML handling code, and general purpose utility functions, so I'm against moving XML handling code into util. I understand why you did this though - the code in storage_backend_fs.c that you later move into util/ depends on it. I think we should refactor the latter though, rather than moving this XML handling code Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Renam virStorageVolFormatFileSystem to virStorageFileFormat and move to src/util/storage_file.[ch] * src/Makefile.am: add src/util/storage_file.[ch] * src/conf/storage_conf.[ch]: move enum from here ... * src/util/storage_file.[ch]: .. to here * src/libvirt_private.syms: update To/FromString exports * src/storage/storage_backend.c, src/storage/storage_backend_fs.c, src/vbox/vbox_tmpl.c: update for above changes --- src/Makefile.am | 1 + src/conf/storage_conf.c | 25 ++++++++------------ src/conf/storage_conf.h | 17 -------------- src/libvirt_private.syms | 5 ++- src/storage/storage_backend.c | 17 +++++++------ src/storage/storage_backend_fs.c | 33 ++++++++++++++------------- src/util/storage_file.c | 31 +++++++++++++++++++++++++ src/util/storage_file.h | 46 ++++++++++++++++++++++++++++++++++++++ src/vbox/vbox_tmpl.c | 15 ++++++----- 9 files changed, 125 insertions(+), 65 deletions(-) create mode 100644 src/util/storage_file.c create mode 100644 src/util/storage_file.h diff --git a/src/Makefile.am b/src/Makefile.am index 5bddb58..9662fd4 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -50,6 +50,7 @@ UTIL_SOURCES = \ util/qparams.c util/qparams.h \ util/stats_linux.c util/stats_linux.h \ util/storage_encryption.c util/storage_encryption.h \ + util/storage_file.c util/storage_file.h \ util/threads.c util/threads.h \ util/threads-pthread.h \ util/threads-win32.h \ diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index cb063cc..788de15 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -36,6 +36,7 @@ #include "virterror_internal.h" #include "datatypes.h" #include "storage_conf.h" +#include "storage_file.h" #include "xml.h" #include "uuid.h" @@ -82,12 +83,6 @@ VIR_ENUM_IMPL(virStorageVolFormatDisk, "linux-lvm", "linux-raid", "extended") -VIR_ENUM_IMPL(virStorageVolFormatFileSystem, - VIR_STORAGE_VOL_FILE_LAST, - "raw", "dir", "bochs", - "cloop", "cow", "dmg", "iso", - "qcow", "qcow2", "vmdk", "vpc") - VIR_ENUM_IMPL(virStoragePartedFsType, VIR_STORAGE_PARTED_FS_TYPE_LAST, "ext2", "ext2", "fat16", @@ -150,9 +145,9 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { }, { .poolType = VIR_STORAGE_POOL_DIR, .volOptions = { - .defaultFormat = VIR_STORAGE_VOL_FILE_RAW, - .formatFromString = virStorageVolFormatFileSystemTypeFromString, - .formatToString = virStorageVolFormatFileSystemTypeToString, + .defaultFormat = VIR_STORAGE_FILE_RAW, + .formatFromString = virStorageFileFormatTypeFromString, + .formatToString = virStorageFileFormatTypeToString, }, }, { .poolType = VIR_STORAGE_POOL_FS, @@ -162,9 +157,9 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { .formatToString = virStoragePoolFormatFileSystemTypeToString, }, .volOptions = { - .defaultFormat = VIR_STORAGE_VOL_FILE_RAW, - .formatFromString = virStorageVolFormatFileSystemTypeFromString, - .formatToString = virStorageVolFormatFileSystemTypeToString, + .defaultFormat = VIR_STORAGE_FILE_RAW, + .formatFromString = virStorageFileFormatTypeFromString, + .formatToString = virStorageFileFormatTypeToString, }, }, { .poolType = VIR_STORAGE_POOL_NETFS, @@ -176,9 +171,9 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { .formatToString = virStoragePoolFormatFileSystemNetTypeToString, }, .volOptions = { - .defaultFormat = VIR_STORAGE_VOL_FILE_RAW, - .formatFromString = virStorageVolFormatFileSystemTypeFromString, - .formatToString = virStorageVolFormatFileSystemTypeToString, + .defaultFormat = VIR_STORAGE_FILE_RAW, + .formatFromString = virStorageFileFormatTypeFromString, + .formatToString = virStorageFileFormatTypeToString, }, }, { .poolType = VIR_STORAGE_POOL_ISCSI, diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 00dd102..7f63fbc 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -429,23 +429,6 @@ enum virStoragePoolFormatLogical { }; VIR_ENUM_DECL(virStoragePoolFormatLogical) - -enum virStorageVolFormatFileSystem { - VIR_STORAGE_VOL_FILE_RAW = 0, - VIR_STORAGE_VOL_FILE_DIR, - VIR_STORAGE_VOL_FILE_BOCHS, - VIR_STORAGE_VOL_FILE_CLOOP, - VIR_STORAGE_VOL_FILE_COW, - VIR_STORAGE_VOL_FILE_DMG, - VIR_STORAGE_VOL_FILE_ISO, - VIR_STORAGE_VOL_FILE_QCOW, - VIR_STORAGE_VOL_FILE_QCOW2, - VIR_STORAGE_VOL_FILE_VMDK, - VIR_STORAGE_VOL_FILE_VPC, - VIR_STORAGE_VOL_FILE_LAST, -}; -VIR_ENUM_DECL(virStorageVolFormatFileSystem) - /* * XXX these are basically partition types. * diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e42a5e4..76a6e1b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -378,8 +378,6 @@ virStorageVolDefParseNode; virStoragePoolFormatDiskTypeToString; virStoragePoolFormatFileSystemTypeToString; virStoragePoolFormatFileSystemNetTypeToString; -virStorageVolFormatFileSystemTypeToString; -virStorageVolFormatFileSystemTypeFromString; virStoragePoolTypeFromString; virStoragePartedFsTypeTypeToString; virStoragePoolObjLock; @@ -392,6 +390,9 @@ virStorageEncryptionParseNode; virStorageEncryptionFormat; virStorageGenerateQcowPassphrase; +# storage_file.h +virStorageFileFormatTypeToString; +virStorageFileFormatTypeFromString; # threads.h virMutexInit; diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index b3c9b0e..18a69be 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -50,6 +50,7 @@ #include "internal.h" #include "secret_conf.h" #include "uuid.h" +#include "storage_file.h" #include "storage_backend.h" #include "logging.h" @@ -461,16 +462,16 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, char *create_tool; short use_kvmimg; - const char *type = virStorageVolFormatFileSystemTypeToString(vol->target.format); + const char *type = virStorageFileFormatTypeToString(vol->target.format); const char *backingType = vol->backingStore.path ? - virStorageVolFormatFileSystemTypeToString(vol->backingStore.format) : NULL; + virStorageFileFormatTypeToString(vol->backingStore.format) : NULL; const char *inputBackingPath = (inputvol ? inputvol->backingStore.path : NULL); const char *inputPath = inputvol ? inputvol->target.path : NULL; /* Treat input block devices as 'raw' format */ const char *inputType = inputPath ? - virStorageVolFormatFileSystemTypeToString(inputvol->type == VIR_STORAGE_VOL_BLOCK ? VIR_STORAGE_VOL_FILE_RAW : inputvol->target.format) : + virStorageFileFormatTypeToString(inputvol->type == VIR_STORAGE_VOL_BLOCK ? VIR_STORAGE_FILE_RAW : inputvol->target.format) : NULL; const char **imgargv; @@ -551,8 +552,8 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, if (vol->target.encryption != NULL) { virStorageEncryptionPtr enc; - if (vol->target.format != VIR_STORAGE_VOL_FILE_QCOW && - vol->target.format != VIR_STORAGE_VOL_FILE_QCOW2) { + if (vol->target.format != VIR_STORAGE_FILE_QCOW && + vol->target.format != VIR_STORAGE_FILE_QCOW2) { virStorageReportError(conn, VIR_ERR_NO_SUPPORT, _("qcow volume encryption unsupported with " "volume format %s"), type); @@ -643,7 +644,7 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn, return -1; } - if (vol->target.format != VIR_STORAGE_VOL_FILE_QCOW2) { + if (vol->target.format != VIR_STORAGE_FILE_QCOW2) { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, _("unsupported storage vol type %d"), vol->target.format); @@ -734,9 +735,9 @@ virStorageBackendGetBuildVolFromFunction(virConnectPtr conn, * tool for converting */ if ((vol->type == VIR_STORAGE_VOL_FILE && - vol->target.format != VIR_STORAGE_VOL_FILE_RAW) || + vol->target.format != VIR_STORAGE_FILE_RAW) || (inputvol->type == VIR_STORAGE_VOL_FILE && - inputvol->target.format != VIR_STORAGE_VOL_FILE_RAW)) { + inputvol->target.format != VIR_STORAGE_FILE_RAW)) { if ((tool_type = virStorageBackendFindFSImageTool(NULL)) < 0) { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 01cb171..e7a3ca9 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -41,6 +41,7 @@ #include "virterror_internal.h" #include "storage_backend_fs.h" #include "storage_conf.h" +#include "storage_file.h" #include "util.h" #include "memory.h" #include "xml.h" @@ -91,7 +92,7 @@ struct FileTypeInfo { struct FileTypeInfo const fileTypeInfo[] = { /* Bochs */ /* XXX Untested - { VIR_STORAGE_VOL_FILE_BOCHS, "Bochs Virtual HD Image", NULL, + { VIR_STORAGE_FILE_BOCHS, "Bochs Virtual HD Image", NULL, LV_LITTLE_ENDIAN, 64, 0x20000, 32+16+16+4+4+4+4+4, 8, 1, -1, NULL },*/ /* CLoop */ @@ -100,45 +101,45 @@ struct FileTypeInfo const fileTypeInfo[] = { LV_LITTLE_ENDIAN, -1, 0, -1, 0, 0, -1, NULL }, */ /* Cow */ - { VIR_STORAGE_VOL_FILE_COW, "OOOM", NULL, + { VIR_STORAGE_FILE_COW, "OOOM", NULL, LV_BIG_ENDIAN, 4, 2, 4+4+1024+4, 8, 1, -1, cowGetBackingStore }, /* DMG */ /* XXX QEMU says there's no magic for dmg, but we should check... */ - { VIR_STORAGE_VOL_FILE_DMG, NULL, ".dmg", + { VIR_STORAGE_FILE_DMG, NULL, ".dmg", 0, -1, 0, -1, 0, 0, -1, NULL }, /* XXX there's probably some magic for iso we can validate too... */ - { VIR_STORAGE_VOL_FILE_ISO, NULL, ".iso", + { VIR_STORAGE_FILE_ISO, NULL, ".iso", 0, -1, 0, -1, 0, 0, -1, NULL }, /* Parallels */ /* XXX Untested - { VIR_STORAGE_VOL_FILE_PARALLELS, "WithoutFreeSpace", NULL, + { VIR_STORAGE_FILE_PARALLELS, "WithoutFreeSpace", NULL, LV_LITTLE_ENDIAN, 16, 2, 16+4+4+4+4, 4, 512, -1, NULL }, */ /* QCow */ - { VIR_STORAGE_VOL_FILE_QCOW, "QFI", NULL, + { VIR_STORAGE_FILE_QCOW, "QFI", NULL, LV_BIG_ENDIAN, 4, 1, 4+4+8+4+4, 8, 1, 4+4+8+4+4+8+1+1+2, qcowXGetBackingStore }, /* QCow 2 */ - { VIR_STORAGE_VOL_FILE_QCOW2, "QFI", NULL, + { VIR_STORAGE_FILE_QCOW2, "QFI", NULL, LV_BIG_ENDIAN, 4, 2, 4+4+8+4+4, 8, 1, 4+4+8+4+4+8, qcowXGetBackingStore }, /* VMDK 3 */ /* XXX Untested - { VIR_STORAGE_VOL_FILE_VMDK, "COWD", NULL, + { VIR_STORAGE_FILE_VMDK, "COWD", NULL, LV_LITTLE_ENDIAN, 4, 1, 4+4+4, 4, 512, -1, NULL }, */ /* VMDK 4 */ - { VIR_STORAGE_VOL_FILE_VMDK, "KDMV", NULL, + { VIR_STORAGE_FILE_VMDK, "KDMV", NULL, LV_LITTLE_ENDIAN, 4, 1, 4+4+4, 8, 512, -1, vmdk4GetBackingStore }, /* Connectix / VirtualPC */ /* XXX Untested - { VIR_STORAGE_VOL_FILE_VPC, "conectix", NULL, + { VIR_STORAGE_FILE_VPC, "conectix", NULL, LV_BIG_ENDIAN, -1, 0, -1, 0, 0, -1, NULL}, */ @@ -452,7 +453,7 @@ static int virStorageBackendProbeTarget(virConnectPtr conn, } /* All fails, so call it a raw file */ - target->format = VIR_STORAGE_VOL_FILE_RAW; + target->format = VIR_STORAGE_FILE_RAW; return 0; } @@ -891,7 +892,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn, goto no_memory; vol->type = VIR_STORAGE_VOL_FILE; - vol->target.format = VIR_STORAGE_VOL_FILE_RAW; /* Real value is filled in during probe */ + vol->target.format = VIR_STORAGE_FILE_RAW; /* Real value is filled in during probe */ if (virAsprintf(&vol->target.path, "%s/%s", pool->def->target.path, vol->name) == -1) @@ -918,7 +919,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn, } if (backingStore != NULL) { - if (vol->target.format == VIR_STORAGE_VOL_FILE_QCOW2 && + if (vol->target.format == VIR_STORAGE_FILE_QCOW2 && STRPREFIX("fmt:", backingStore)) { char *fmtstr = backingStore + 4; char *path = strchr(fmtstr, ':'); @@ -927,7 +928,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn, } else { *path = '\0'; if ((vol->backingStore.format = - virStorageVolFormatFileSystemTypeFromString(fmtstr)) < 0) { + virStorageFileFormatTypeFromString(fmtstr)) < 0) { VIR_FREE(backingStore); } else { memmove(backingStore, path, strlen(path) + 1); @@ -1121,9 +1122,9 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn, inputvol); if (!create_func) return -1; - } else if (vol->target.format == VIR_STORAGE_VOL_FILE_RAW) { + } else if (vol->target.format == VIR_STORAGE_FILE_RAW) { create_func = virStorageBackendCreateRaw; - } else if (vol->target.format == VIR_STORAGE_VOL_FILE_DIR) { + } else if (vol->target.format == VIR_STORAGE_FILE_DIR) { create_func = createFileDir; } else if ((tool_type = virStorageBackendFindFSImageTool(NULL)) != -1) { create_func = virStorageBackendFSImageToolTypeToFunc(conn, tool_type); diff --git a/src/util/storage_file.c b/src/util/storage_file.c new file mode 100644 index 0000000..e66ec8a --- /dev/null +++ b/src/util/storage_file.c @@ -0,0 +1,31 @@ +/* + * storage_file.c: file utility functions for FS storage backend + * + * Copyright (C) 2007-2009 Red Hat, Inc. + * Copyright (C) 2007-2008 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#include <config.h> +#include "storage_file.h" + +VIR_ENUM_IMPL(virStorageFileFormat, + VIR_STORAGE_FILE_LAST, + "raw", "dir", "bochs", + "cloop", "cow", "dmg", "iso", + "qcow", "qcow2", "vmdk", "vpc") diff --git a/src/util/storage_file.h b/src/util/storage_file.h new file mode 100644 index 0000000..7bccbe4 --- /dev/null +++ b/src/util/storage_file.h @@ -0,0 +1,46 @@ +/* + * storage_file.c: file utility functions for FS storage backend + * + * Copyright (C) 2007-2009 Red Hat, Inc. + * Copyright (C) 2007-2008 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#ifndef __VIR_STORAGE_FILE_H__ +#define __VIR_STORAGE_FILE_H__ + +#include "util.h" + +enum virStorageFileFormat { + VIR_STORAGE_FILE_RAW = 0, + VIR_STORAGE_FILE_DIR, + VIR_STORAGE_FILE_BOCHS, + VIR_STORAGE_FILE_CLOOP, + VIR_STORAGE_FILE_COW, + VIR_STORAGE_FILE_DMG, + VIR_STORAGE_FILE_ISO, + VIR_STORAGE_FILE_QCOW, + VIR_STORAGE_FILE_QCOW2, + VIR_STORAGE_FILE_VMDK, + VIR_STORAGE_FILE_VPC, + VIR_STORAGE_FILE_LAST, +}; + +VIR_ENUM_DECL(virStorageFileFormat); + +#endif /* __VIR_STORAGE_FILE_H__ */ diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index c02b18c..ecad2a1 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -45,6 +45,7 @@ #include "virterror_internal.h" #include "domain_event.h" #include "storage_conf.h" +#include "storage_file.h" #include "uuid.h" #include "event.h" #include "memory.h" @@ -5980,14 +5981,14 @@ static virStorageVolPtr vboxStorageVolCreateXML(virStoragePoolPtr pool, /* TODO: for now only the vmdk, vpc and vdi type harddisk * variants can be created, also since there is no vdi - * type in enum virStorageVolFormatFileSystem {} the default + * type in enum virStorageFileFormat {} the default * will be to create vdi if nothing is specified in * def->target.format */ - if (def->target.format == VIR_STORAGE_VOL_FILE_VMDK) { + if (def->target.format == VIR_STORAGE_FILE_VMDK) { data->pFuncs->pfnUtf8ToUtf16("VMDK", &hddFormatUtf16); - } else if (def->target.format == VIR_STORAGE_VOL_FILE_VPC) { + } else if (def->target.format == VIR_STORAGE_FILE_VPC) { data->pFuncs->pfnUtf8ToUtf16("VHD", &hddFormatUtf16); } else { data->pFuncs->pfnUtf8ToUtf16("VDI", &hddFormatUtf16); @@ -6302,13 +6303,13 @@ static char *vboxStorageVolGetXMLDesc(virStorageVolPtr vol, unsigned int flags A DEBUG("Storage Volume Format: %s", hddFormatUtf8); if (STRCASEEQ("vmdk", hddFormatUtf8)) - def.target.format = VIR_STORAGE_VOL_FILE_VMDK; + def.target.format = VIR_STORAGE_FILE_VMDK; else if (STRCASEEQ("vhd", hddFormatUtf8)) - def.target.format = VIR_STORAGE_VOL_FILE_VPC; + def.target.format = VIR_STORAGE_FILE_VPC; else - def.target.format = VIR_STORAGE_VOL_FILE_RAW; + def.target.format = VIR_STORAGE_FILE_RAW; - /* TODO: need to add vdi to enum virStorageVolFormatFileSystem {} + /* TODO: need to add vdi to enum virStorageFileFormat {} * and then add it here */ -- 1.6.2.5

On Fri, Sep 25, 2009 at 02:27:29PM +0100, Mark McLoughlin wrote:
Renam virStorageVolFormatFileSystem to virStorageFileFormat and move to src/util/storage_file.[ch]
* src/Makefile.am: add src/util/storage_file.[ch]
* src/conf/storage_conf.[ch]: move enum from here ...
* src/util/storage_file.[ch]: .. to here
* src/libvirt_private.syms: update To/FromString exports
* src/storage/storage_backend.c, src/storage/storage_backend_fs.c, src/vbox/vbox_tmpl.c: update for above changes --- src/Makefile.am | 1 + src/conf/storage_conf.c | 25 ++++++++------------ src/conf/storage_conf.h | 17 -------------- src/libvirt_private.syms | 5 ++- src/storage/storage_backend.c | 17 +++++++------ src/storage/storage_backend_fs.c | 33 ++++++++++++++------------- src/util/storage_file.c | 31 +++++++++++++++++++++++++ src/util/storage_file.h | 46 ++++++++++++++++++++++++++++++++++++++ src/vbox/vbox_tmpl.c | 15 ++++++----- 9 files changed, 125 insertions(+), 65 deletions(-) create mode 100644 src/util/storage_file.c create mode 100644 src/util/storage_file.h
ACK, this is fine Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Rename to virStorageFilePerms and move to util/storage_file.[ch] * src/conf/storage_conf.h: move perms struct from here ... * src/util/storage_file.h: ... to here * src/conf/storage_conf.c: update --- src/conf/storage_conf.c | 2 +- src/conf/storage_conf.h | 18 +++--------------- src/util/storage_file.h | 9 +++++++++ 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 788de15..dc6171c 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -387,7 +387,7 @@ virStoragePoolDefParseAuthChap(virConnectPtr conn, static int virStorageDefParsePerms(virConnectPtr conn, xmlXPathContextPtr ctxt, - virStoragePermsPtr perms, + virStorageFilePermsPtr perms, const char *permxpath, int defaultmode) { char *mode; diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 7f63fbc..9353cf5 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -27,23 +27,11 @@ #include "internal.h" #include "util.h" #include "storage_encryption.h" +#include "storage_file.h" #include "threads.h" #include <libxml/tree.h> -/* Shared structs */ - - - -typedef struct _virStoragePerms virStoragePerms; -typedef virStoragePerms *virStoragePermsPtr; -struct _virStoragePerms { - int mode; - int uid; - int gid; - char *label; -}; - /* Storage volumes */ @@ -76,7 +64,7 @@ typedef virStorageVolTarget *virStorageVolTargetPtr; struct _virStorageVolTarget { char *path; int format; - virStoragePerms perms; + virStorageFilePerms perms; int type; /* only used by disk backend for partition type */ /* Currently used only in virStorageVolDef.target, not in .backingstore. */ virStorageEncryptionPtr encryption; @@ -236,7 +224,7 @@ typedef struct _virStoragePoolTarget virStoragePoolTarget; typedef virStoragePoolTarget *virStoragePoolTargetPtr; struct _virStoragePoolTarget { char *path; /* Optional local filesystem mapping */ - virStoragePerms perms; /* Default permissions for volumes */ + virStorageFilePerms perms; /* Default permissions for volumes */ }; diff --git a/src/util/storage_file.h b/src/util/storage_file.h index 7bccbe4..e2f1478 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -43,4 +43,13 @@ enum virStorageFileFormat { VIR_ENUM_DECL(virStorageFileFormat); +typedef struct _virStorageFilePerms virStorageFilePerms; +typedef virStorageFilePerms *virStorageFilePermsPtr; +struct _virStorageFilePerms { + int mode; + int uid; + int gid; + char *label; +}; + #endif /* __VIR_STORAGE_FILE_H__ */ -- 1.6.2.5

On Fri, Sep 25, 2009 at 02:27:30PM +0100, Mark McLoughlin wrote:
Rename to virStorageFilePerms and move to util/storage_file.[ch]
* src/conf/storage_conf.h: move perms struct from here ...
* src/util/storage_file.h: ... to here
* src/conf/storage_conf.c: update --- src/conf/storage_conf.c | 2 +- src/conf/storage_conf.h | 18 +++--------------- src/util/storage_file.h | 9 +++++++++ 3 files changed, 13 insertions(+), 16 deletions(-)
NACK, i think this is redundant if we re-factor the file probing code just a little. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Rename virStorageBackendUpdateVolTargetInfo to virStorageFileGetInfo() and move to util/storage_file.[ch] * src/storage/storage_backend.[ch]: move code from here ... * src/util/storage_file.[ch]: ... to here * src/libvirt_private.syms: export new functions * src/storage/storage_backend_fs.c, src/storage/storage_backend_mpath.c, src/storage/storage_backend_scsi.c: update from above changes * po/POTFILES.in: add storage_file.c --- po/POTFILES.in | 1 + src/libvirt_private.syms | 2 + src/storage/storage_backend.c | 148 ++------------------------------- src/storage/storage_backend.h | 9 -- src/storage/storage_backend_fs.c | 18 ++-- src/storage/storage_backend_mpath.c | 7 +- src/storage/storage_backend_scsi.c | 7 +- src/util/storage_file.c | 157 +++++++++++++++++++++++++++++++++++ src/util/storage_file.h | 13 +++ 9 files changed, 194 insertions(+), 168 deletions(-) diff --git a/po/POTFILES.in b/po/POTFILES.in index 9f21459..30c52f5 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -45,6 +45,7 @@ src/util/iptables.c src/util/logging.c src/util/pci.c src/util/storage_encryption.c +src/util/storage_file.c src/util/util.c src/util/uuid.c src/util/virterror.c diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 76a6e1b..9b6f4a7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -393,6 +393,8 @@ virStorageGenerateQcowPassphrase; # storage_file.h virStorageFileFormatTypeToString; virStorageFileFormatTypeFromString; +virStorageFileGetInfo; +virStorageFileGetInfoFromFD; # threads.h virMutexInit; diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 18a69be..e214c3f 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -73,10 +73,6 @@ #include "storage_backend_fs.h" #endif -#ifndef DEV_BSIZE -#define DEV_BSIZE 512 -#endif - #define VIR_FROM_THIS VIR_FROM_STORAGE static virStorageBackendPtr backends[] = { @@ -768,33 +764,6 @@ virStorageBackendForType(int type) { return NULL; } - -int -virStorageBackendUpdateVolTargetInfo(virConnectPtr conn, - virStorageVolTargetPtr target, - unsigned long long *allocation, - unsigned long long *capacity) -{ - int ret, fd; - - if ((fd = open(target->path, O_RDONLY)) < 0) { - virReportSystemError(conn, errno, - _("cannot open volume '%s'"), - target->path); - return -1; - } - - ret = virStorageBackendUpdateVolTargetInfoFD(conn, - target, - fd, - allocation, - capacity); - - close(fd); - - return ret; -} - int virStorageBackendUpdateVolInfo(virConnectPtr conn, virStorageVolDefPtr vol, @@ -802,122 +771,23 @@ virStorageBackendUpdateVolInfo(virConnectPtr conn, { int ret; - if ((ret = virStorageBackendUpdateVolTargetInfo(conn, - &vol->target, - &vol->allocation, - withCapacity ? &vol->capacity : NULL)) < 0) + if ((ret = virStorageFileGetInfo(conn, + vol->target.path, + &vol->target.perms, + &vol->allocation, + withCapacity ? &vol->capacity : NULL)) < 0) return ret; if (vol->backingStore.path && - (ret = virStorageBackendUpdateVolTargetInfo(conn, - &vol->backingStore, - NULL, NULL)) < 0) + (ret = virStorageFileGetInfo(conn, + vol->backingStore.path, + &vol->backingStore.perms, + NULL, NULL)) < 0) return ret; return 0; } -/* - * virStorageBackendUpdateVolTargetInfoFD: - * @conn: connection to report errors on - * @target: target definition ptr of volume to update - * @fd: fd of storage volume to update - * @allocation: If not NULL, updated allocation information will be stored - * @capacity: If not NULL, updated capacity info will be stored - * - * Returns 0 for success-1 on a legitimate error condition, - * -2 if passed FD isn't a regular, char, or block file. - */ -int -virStorageBackendUpdateVolTargetInfoFD(virConnectPtr conn, - virStorageVolTargetPtr target, - int fd, - unsigned long long *allocation, - unsigned long long *capacity) -{ - struct stat sb; -#if HAVE_SELINUX - security_context_t filecon = NULL; -#endif - - if (fstat(fd, &sb) < 0) { - virReportSystemError(conn, errno, - _("cannot stat file '%s'"), - target->path); - return -1; - } - - if (!S_ISREG(sb.st_mode) && - !S_ISCHR(sb.st_mode) && - !S_ISBLK(sb.st_mode)) - return -2; - - if (allocation) { - if (S_ISREG(sb.st_mode)) { -#ifndef __MINGW32__ - *allocation = (unsigned long long)sb.st_blocks * - (unsigned long long)DEV_BSIZE; -#else - *allocation = sb.st_size; -#endif - /* Regular files may be sparse, so logical size (capacity) is not same - * as actual allocation above - */ - if (capacity) - *capacity = sb.st_size; - } else { - off_t end; - /* XXX this is POSIX compliant, but doesn't work for for CHAR files, - * only BLOCK. There is a Linux specific ioctl() for getting - * size of both CHAR / BLOCK devices we should check for in - * configure - */ - end = lseek(fd, 0, SEEK_END); - if (end == (off_t)-1) { - virReportSystemError(conn, errno, - _("cannot seek to end of file '%s'"), - target->path); - return -1; - } - *allocation = end; - if (capacity) - *capacity = end; - } - } - - target->perms.mode = sb.st_mode & S_IRWXUGO; - target->perms.uid = sb.st_uid; - target->perms.gid = sb.st_gid; - - VIR_FREE(target->perms.label); - -#if HAVE_SELINUX - /* XXX: make this a security driver call */ - if (fgetfilecon(fd, &filecon) == -1) { - if (errno != ENODATA && errno != ENOTSUP) { - virReportSystemError(conn, errno, - _("cannot get file context of '%s'"), - target->path); - return -1; - } else { - target->perms.label = NULL; - } - } else { - target->perms.label = strdup(filecon); - if (target->perms.label == NULL) { - virReportOOMError(conn); - return -1; - } - freecon(filecon); - } -#else - target->perms.label = NULL; -#endif - - return 0; -} - - struct diskType { int part_table_type; unsigned short offset; diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 88c6161..550bd32 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -82,15 +82,6 @@ int virStorageBackendUpdateVolInfo(virConnectPtr conn, virStorageVolDefPtr vol, int withCapacity); -int virStorageBackendUpdateVolTargetInfo(virConnectPtr conn, - virStorageVolTargetPtr target, - unsigned long long *allocation, - unsigned long long *capacity); -int virStorageBackendUpdateVolTargetInfoFD(virConnectPtr conn, - virStorageVolTargetPtr target, - int fd, - unsigned long long *allocation, - unsigned long long *capacity); int virStorageBackendUpdateVolTargetFormatFD(virConnectPtr conn, virStorageVolTargetPtr target, diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index e7a3ca9..49cfc6f 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -305,9 +305,9 @@ static int virStorageBackendProbeTarget(virConnectPtr conn, return -1; } - if ((ret = virStorageBackendUpdateVolTargetInfoFD(conn, target, fd, - allocation, - capacity)) < 0) { + if ((ret = virStorageFileGetInfoFromFD(conn, target->path, fd, + &target->perms, + allocation, capacity)) < 0) { close(fd); return ret; /* Take care to propagate ret, it is not always -1 */ } @@ -934,10 +934,8 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn, memmove(backingStore, path, strlen(path) + 1); vol->backingStore.path = backingStore; - if (virStorageBackendUpdateVolTargetInfo(conn, - &vol->backingStore, - NULL, - NULL) < 0) + if (virStorageFileGetInfo(conn, vol->backingStore.path, + &vol->backingStore.perms, NULL, NULL) < 0) VIR_FREE(vol->backingStore); } } @@ -1167,9 +1165,9 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn, } /* Refresh allocation / permissions info, but not capacity */ - if (virStorageBackendUpdateVolTargetInfoFD(conn, &vol->target, fd, - &vol->allocation, - NULL) < 0) { + if (virStorageFileGetInfoFromFD(conn, vol->target.path, fd, + &vol->target.perms, &vol->allocation, + NULL) < 0) { close(fd); return -1; } diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index fcc38ba..71e86d0 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -55,11 +55,8 @@ virStorageBackendMpathUpdateVolTargetInfo(virConnectPtr conn, goto out; } - if (virStorageBackendUpdateVolTargetInfoFD(conn, - target, - fd, - allocation, - capacity) < 0) { + if (virStorageFileGetInfoFromFD(conn, target->path, fd, &target->perms, + allocation, capacity) < 0) { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, _("Failed to update volume target info for '%s'"), diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index c70b1ed..f9eb0ab 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -149,11 +149,8 @@ virStorageBackendSCSIUpdateVolTargetInfo(virConnectPtr conn, return -1; } - if (virStorageBackendUpdateVolTargetInfoFD(conn, - target, - fd, - allocation, - capacity) < 0) + if (virStorageFileGetInfoFromFD(conn, target->path, fd, &target->perms, + allocation, capacity) < 0) goto cleanup; /* make sure to set the target format "unknown" to begin with */ diff --git a/src/util/storage_file.c b/src/util/storage_file.c index e66ec8a..c1eec9b 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -24,8 +24,165 @@ #include <config.h> #include "storage_file.h" +#include <sys/statvfs.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <errno.h> +#include <fcntl.h> +#include <unistd.h> +#include <string.h> + +#if HAVE_SELINUX +#include <selinux/selinux.h> +#endif + +#include "memory.h" +#include "virterror_internal.h" + +#define VIR_FROM_THIS VIR_FROM_STORAGE + VIR_ENUM_IMPL(virStorageFileFormat, VIR_STORAGE_FILE_LAST, "raw", "dir", "bochs", "cloop", "cow", "dmg", "iso", "qcow", "qcow2", "vmdk", "vpc") + +#ifndef DEV_BSIZE +#define DEV_BSIZE 512 +#endif + +/* + * virStorageFileGetInfoFromFD: + * @conn: connection to report errors on + * @path: path of the file + * @fd: file description of the open file + * @permissions: If not NULL, file permsions info to update + * @allocation: If not NULL, updated allocation information will be stored + * @capacity: If not NULL, updated capacity info will be stored + * + * Returns 0 for success-1 on a legitimate error condition, + * -2 if passed FD isn't a regular, char, or block file. + */ +int +virStorageFileGetInfoFromFD(virConnectPtr conn, + const char *path, + int fd, + virStorageFilePermsPtr perms, + unsigned long long *allocation, + unsigned long long *capacity) +{ + struct stat sb; +#if HAVE_SELINUX + security_context_t filecon = NULL; +#endif + + if (fstat(fd, &sb) < 0) { + virReportSystemError(conn, errno, + _("cannot stat file '%s'"), path); + return -1; + } + + if (!S_ISREG(sb.st_mode) && + !S_ISCHR(sb.st_mode) && + !S_ISBLK(sb.st_mode)) + return -2; + + if (allocation) { + if (S_ISREG(sb.st_mode)) { +#ifndef __MINGW32__ + *allocation = (unsigned long long)sb.st_blocks * + (unsigned long long)DEV_BSIZE; +#else + *allocation = sb.st_size; +#endif + /* Regular files may be sparse, so logical size (capacity) is not same + * as actual allocation above + */ + if (capacity) + *capacity = sb.st_size; + } else { + off_t end; + /* XXX this is POSIX compliant, but doesn't work for for CHAR files, + * only BLOCK. There is a Linux specific ioctl() for getting + * size of both CHAR / BLOCK devices we should check for in + * configure + */ + end = lseek(fd, 0, SEEK_END); + if (end == (off_t)-1) { + virReportSystemError(conn, errno, + _("cannot seek to end of file '%s'"), path); + return -1; + } + *allocation = end; + if (capacity) + *capacity = end; + } + } + + if (!perms) + return 0; + + perms->mode = sb.st_mode & S_IRWXUGO; + perms->uid = sb.st_uid; + perms->gid = sb.st_gid; + + VIR_FREE(perms->label); + +#if HAVE_SELINUX + /* XXX: make this a security driver call */ + if (fgetfilecon(fd, &filecon) == -1) { + if (errno != ENODATA && errno != ENOTSUP) { + virReportSystemError(conn, errno, + _("cannot get file context of '%s'"), path); + return -1; + } else { + perms->label = NULL; + } + } else { + perms->label = strdup(filecon); + if (perms->label == NULL) { + virReportOOMError(conn); + return -1; + } + freecon(filecon); + } +#else + perms->label = NULL; +#endif + + return 0; +} + +/* + * virFileGetInfo: + * + * @path: path of the file + * @perms: If not NULL, file permissions information will be stored + * @allocation: If not NULL, file allocation information will be stored + * @capacity: If not NULL, file capacity info will be stored + * + * Returns 0 for success-1 on a legitimate error condition, + * -2 if passed FD isn't a regular, char, or block file. + */ +int +virStorageFileGetInfo(virConnectPtr conn, + const char *path, + virStorageFilePermsPtr perms, + unsigned long long *allocation, + unsigned long long *capacity) +{ + int ret, fd; + + if ((fd = open(path, O_RDONLY)) < 0) { + virReportSystemError(conn, errno, + _("cannot open volume '%s'"), path); + return -1; + } + + ret = virStorageFileGetInfoFromFD(conn, path, fd, perms, + allocation, capacity); + + close(fd); + + return ret; +} diff --git a/src/util/storage_file.h b/src/util/storage_file.h index e2f1478..efacc65 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -52,4 +52,17 @@ struct _virStorageFilePerms { char *label; }; +int virStorageFileGetInfo(virConnectPtr conn, + const char *path, + virStorageFilePermsPtr perms, + unsigned long long *allocation, + unsigned long long *capacity); + +int virStorageFileGetInfoFromFD(virConnectPtr conn, + const char *path, + int fd, + virStorageFilePermsPtr perms, + unsigned long long *allocation, + unsigned long long *capacity); + #endif /* __VIR_STORAGE_FILE_H__ */ -- 1.6.2.5

On Fri, Sep 25, 2009 at 02:27:31PM +0100, Mark McLoughlin wrote:
Rename virStorageBackendUpdateVolTargetInfo to virStorageFileGetInfo() and move to util/storage_file.[ch]
* src/storage/storage_backend.[ch]: move code from here ...
* src/util/storage_file.[ch]: ... to here
* src/libvirt_private.syms: export new functions
* src/storage/storage_backend_fs.c, src/storage/storage_backend_mpath.c, src/storage/storage_backend_scsi.c: update from above changes
* po/POTFILES.in: add storage_file.c --- po/POTFILES.in | 1 + src/libvirt_private.syms | 2 + src/storage/storage_backend.c | 148 ++------------------------------- src/storage/storage_backend.h | 9 -- src/storage/storage_backend_fs.c | 18 ++-- src/storage/storage_backend_mpath.c | 7 +- src/storage/storage_backend_scsi.c | 7 +- src/util/storage_file.c | 157 +++++++++++++++++++++++++++++++++++ src/util/storage_file.h | 13 +++ 9 files changed, 194 insertions(+), 168 deletions(-)
NACK to this one too, based suggestion against patch 6
- - target->perms.mode = sb.st_mode & S_IRWXUGO; - target->perms.uid = sb.st_uid; - target->perms.gid = sb.st_gid; - - VIR_FREE(target->perms.label); - -#if HAVE_SELINUX - /* XXX: make this a security driver call */ - if (fgetfilecon(fd, &filecon) == -1) { - if (errno != ENODATA && errno != ENOTSUP) { - virReportSystemError(conn, errno, - _("cannot get file context of '%s'"), - target->path); - return -1; - } else { - target->perms.label = NULL; - } - } else { - target->perms.label = strdup(filecon); - if (target->perms.label == NULL) { - virReportOOMError(conn); - return -1; - } - freecon(filecon); - } -#else - target->perms.label = NULL; -#endif
This bit of code / todo item is another good argument against moving it - we need to eventually mak this call into the security driver, and don't want to have to move the security drivers into the src/util/ directory / library too. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Finally, we get to the point of all this. Rename virStorageBackendProbeTarget() to virStorageFileProbeHeader() and move to src/util/storage_file.[ch] * src/storage/storage_backend_fs.c: move code from here ... * src/util/storage_file.[ch]: ... to here * src/libvirt_private.syms: export virStorageFileProbeHeader() --- src/libvirt_private.syms | 1 + src/storage/storage_backend_fs.c | 435 ++------------------------------------ src/util/storage_file.c | 409 +++++++++++++++++++++++++++++++++++ src/util/storage_file.h | 10 + 4 files changed, 435 insertions(+), 420 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9b6f4a7..6a353ee 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -395,6 +395,7 @@ virStorageFileFormatTypeToString; virStorageFileFormatTypeFromString; virStorageFileGetInfo; virStorageFileGetInfoFromFD; +virStorageFileProbeHeader; # threads.h virMutexInit; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 49cfc6f..fdc12c3 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -46,419 +46,10 @@ #include "memory.h" #include "xml.h" -enum lv_endian { - LV_LITTLE_ENDIAN = 1, /* 1234 */ - LV_BIG_ENDIAN /* 4321 */ -}; - -enum { - BACKING_STORE_OK, - BACKING_STORE_INVALID, - BACKING_STORE_ERROR, -}; - -static int cowGetBackingStore(virConnectPtr, char **, - const unsigned char *, size_t); -static int qcowXGetBackingStore(virConnectPtr, char **, - const unsigned char *, size_t); -static int vmdk4GetBackingStore(virConnectPtr, char **, - const unsigned char *, size_t); - -/* Either 'magic' or 'extension' *must* be provided */ -struct FileTypeInfo { - int type; /* One of the constants above */ - const char *magic; /* Optional string of file magic - * to check at head of file */ - const char *extension; /* Optional file extension to check */ - enum lv_endian endian; /* Endianness of file format */ - int versionOffset; /* Byte offset from start of file - * where we find version number, - * -1 to skip version test */ - int versionNumber; /* Version number to validate */ - int sizeOffset; /* Byte offset from start of file - * where we find capacity info, - * -1 to use st_size as capacity */ - int sizeBytes; /* Number of bytes for size field */ - int sizeMultiplier; /* A scaling factor if size is not in bytes */ - /* Store a COW base image path (possibly relative), - * or NULL if there is no COW base image, to RES; - * return BACKING_STORE_* */ - int qcowCryptOffset; /* Byte offset from start of file - * where to find encryption mode, - * -1 if encryption is not used */ - int (*getBackingStore)(virConnectPtr conn, char **res, - const unsigned char *buf, size_t buf_size); -}; -struct FileTypeInfo const fileTypeInfo[] = { - /* Bochs */ - /* XXX Untested - { VIR_STORAGE_FILE_BOCHS, "Bochs Virtual HD Image", NULL, - LV_LITTLE_ENDIAN, 64, 0x20000, - 32+16+16+4+4+4+4+4, 8, 1, -1, NULL },*/ - /* CLoop */ - /* XXX Untested - { VIR_STORAGE_VOL_CLOOP, "#!/bin/sh\n#V2.0 Format\nmodprobe cloop file=$0 && mount -r -t iso9660 /dev/cloop $1\n", NULL, - LV_LITTLE_ENDIAN, -1, 0, - -1, 0, 0, -1, NULL }, */ - /* Cow */ - { VIR_STORAGE_FILE_COW, "OOOM", NULL, - LV_BIG_ENDIAN, 4, 2, - 4+4+1024+4, 8, 1, -1, cowGetBackingStore }, - /* DMG */ - /* XXX QEMU says there's no magic for dmg, but we should check... */ - { VIR_STORAGE_FILE_DMG, NULL, ".dmg", - 0, -1, 0, - -1, 0, 0, -1, NULL }, - /* XXX there's probably some magic for iso we can validate too... */ - { VIR_STORAGE_FILE_ISO, NULL, ".iso", - 0, -1, 0, - -1, 0, 0, -1, NULL }, - /* Parallels */ - /* XXX Untested - { VIR_STORAGE_FILE_PARALLELS, "WithoutFreeSpace", NULL, - LV_LITTLE_ENDIAN, 16, 2, - 16+4+4+4+4, 4, 512, -1, NULL }, - */ - /* QCow */ - { VIR_STORAGE_FILE_QCOW, "QFI", NULL, - LV_BIG_ENDIAN, 4, 1, - 4+4+8+4+4, 8, 1, 4+4+8+4+4+8+1+1+2, qcowXGetBackingStore }, - /* QCow 2 */ - { VIR_STORAGE_FILE_QCOW2, "QFI", NULL, - LV_BIG_ENDIAN, 4, 2, - 4+4+8+4+4, 8, 1, 4+4+8+4+4+8, qcowXGetBackingStore }, - /* VMDK 3 */ - /* XXX Untested - { VIR_STORAGE_FILE_VMDK, "COWD", NULL, - LV_LITTLE_ENDIAN, 4, 1, - 4+4+4, 4, 512, -1, NULL }, - */ - /* VMDK 4 */ - { VIR_STORAGE_FILE_VMDK, "KDMV", NULL, - LV_LITTLE_ENDIAN, 4, 1, - 4+4+4, 8, 512, -1, vmdk4GetBackingStore }, - /* Connectix / VirtualPC */ - /* XXX Untested - { VIR_STORAGE_FILE_VPC, "conectix", NULL, - LV_BIG_ENDIAN, -1, 0, - -1, 0, 0, -1, NULL}, - */ -}; +#if WITH_STORAGE_FS #define VIR_FROM_THIS VIR_FROM_STORAGE -static int -cowGetBackingStore(virConnectPtr conn, - char **res, - const unsigned char *buf, - size_t buf_size) -{ -#define COW_FILENAME_MAXLEN 1024 - *res = NULL; - if (buf_size < 4+4+ COW_FILENAME_MAXLEN) - return BACKING_STORE_INVALID; - if (buf[4+4] == '\0') /* cow_header_v2.backing_file[0] */ - return BACKING_STORE_OK; - - *res = strndup ((const char*)buf + 4+4, COW_FILENAME_MAXLEN); - if (*res == NULL) { - virReportOOMError(conn); - return BACKING_STORE_ERROR; - } - return BACKING_STORE_OK; -} - -static int -qcowXGetBackingStore(virConnectPtr conn, - char **res, - const unsigned char *buf, - size_t buf_size) -{ - unsigned long long offset; - unsigned long size; - - *res = NULL; - if (buf_size < 4+4+8+4) - return BACKING_STORE_INVALID; - offset = (((unsigned long long)buf[4+4] << 56) - | ((unsigned long long)buf[4+4+1] << 48) - | ((unsigned long long)buf[4+4+2] << 40) - | ((unsigned long long)buf[4+4+3] << 32) - | ((unsigned long long)buf[4+4+4] << 24) - | ((unsigned long long)buf[4+4+5] << 16) - | ((unsigned long long)buf[4+4+6] << 8) - | buf[4+4+7]); /* QCowHeader.backing_file_offset */ - if (offset > buf_size) - return BACKING_STORE_INVALID; - size = ((buf[4+4+8] << 24) - | (buf[4+4+8+1] << 16) - | (buf[4+4+8+2] << 8) - | buf[4+4+8+3]); /* QCowHeader.backing_file_size */ - if (size == 0) - return BACKING_STORE_OK; - if (offset + size > buf_size || offset + size < offset) - return BACKING_STORE_INVALID; - if (size + 1 == 0) - return BACKING_STORE_INVALID; - if (VIR_ALLOC_N(*res, size + 1) < 0) { - virReportOOMError(conn); - return BACKING_STORE_ERROR; - } - memcpy(*res, buf + offset, size); - (*res)[size] = '\0'; - return BACKING_STORE_OK; -} - - -static int -vmdk4GetBackingStore(virConnectPtr conn, - char **res, - const unsigned char *buf, - size_t buf_size) -{ - static const char prefix[] = "parentFileNameHint=\""; - - char desc[20*512 + 1], *start, *end; - size_t len; - - *res = NULL; - - if (buf_size <= 0x200) - return BACKING_STORE_INVALID; - len = buf_size - 0x200; - if (len > sizeof(desc) - 1) - len = sizeof(desc) - 1; - memcpy(desc, buf + 0x200, len); - desc[len] = '\0'; - start = strstr(desc, prefix); - if (start == NULL) - return BACKING_STORE_OK; - start += strlen(prefix); - end = strchr(start, '"'); - if (end == NULL) - return BACKING_STORE_INVALID; - if (end == start) - return BACKING_STORE_OK; - *end = '\0'; - *res = strdup(start); - if (*res == NULL) { - virReportOOMError(conn); - return BACKING_STORE_ERROR; - } - return BACKING_STORE_OK; -} - -/** - * Return an absolute path corresponding to PATH, which is absolute or relative - * to the directory containing BASE_FILE, or NULL on error - */ -static char *absolutePathFromBaseFile(const char *base_file, const char *path) -{ - size_t base_size, path_size; - char *res, *p; - - if (*path == '/') - return strdup(path); - - base_size = strlen(base_file) + 1; - path_size = strlen(path) + 1; - if (VIR_ALLOC_N(res, base_size - 1 + path_size) < 0) - return NULL; - memcpy(res, base_file, base_size); - p = strrchr(res, '/'); - if (p != NULL) - p++; - else - p = res; - memcpy(p, path, path_size); - if (VIR_REALLOC_N(res, (p + path_size) - res) < 0) { - /* Ignore failure */ - } - return res; -} - - - -/** - * Probe the header of a file to determine what type of disk image - * it is, and info about its capacity if available. - */ -static int virStorageBackendProbeTarget(virConnectPtr conn, - virStorageVolTargetPtr target, - char **backingStore, - unsigned long long *allocation, - unsigned long long *capacity, - virStorageEncryptionPtr *encryption) { - int fd; - unsigned char head[20*512]; /* vmdk4GetBackingStore needs this much. */ - int len, i, ret; - - if (backingStore) - *backingStore = NULL; - if (encryption) - *encryption = NULL; - - if ((fd = open(target->path, O_RDONLY)) < 0) { - virReportSystemError(conn, errno, - _("cannot open volume '%s'"), - target->path); - return -1; - } - - if ((ret = virStorageFileGetInfoFromFD(conn, target->path, fd, - &target->perms, - allocation, capacity)) < 0) { - close(fd); - return ret; /* Take care to propagate ret, it is not always -1 */ - } - - if ((len = read(fd, head, sizeof(head))) < 0) { - virReportSystemError(conn, errno, - _("cannot read header '%s'"), - target->path); - close(fd); - return -1; - } - - close(fd); - - /* First check file magic */ - for (i = 0 ; i < ARRAY_CARDINALITY(fileTypeInfo) ; i++) { - int mlen; - bool encrypted_qcow = false; - - if (fileTypeInfo[i].magic == NULL) - continue; - - /* Validate magic data */ - mlen = strlen(fileTypeInfo[i].magic); - if (mlen > len) - continue; - if (memcmp(head, fileTypeInfo[i].magic, mlen) != 0) - continue; - - /* Validate version number info */ - if (fileTypeInfo[i].versionNumber != -1) { - int version; - - if (fileTypeInfo[i].endian == LV_LITTLE_ENDIAN) { - version = (head[fileTypeInfo[i].versionOffset+3] << 24) | - (head[fileTypeInfo[i].versionOffset+2] << 16) | - (head[fileTypeInfo[i].versionOffset+1] << 8) | - head[fileTypeInfo[i].versionOffset]; - } else { - version = (head[fileTypeInfo[i].versionOffset] << 24) | - (head[fileTypeInfo[i].versionOffset+1] << 16) | - (head[fileTypeInfo[i].versionOffset+2] << 8) | - head[fileTypeInfo[i].versionOffset+3]; - } - if (version != fileTypeInfo[i].versionNumber) - continue; - } - - /* Optionally extract capacity from file */ - if (fileTypeInfo[i].sizeOffset != -1 && capacity) { - if (fileTypeInfo[i].endian == LV_LITTLE_ENDIAN) { - *capacity = - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7] << 56) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+6] << 48) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+5] << 40) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+4] << 32) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+3] << 24) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+2] << 16) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+1] << 8) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset]); - } else { - *capacity = - ((unsigned long long)head[fileTypeInfo[i].sizeOffset] << 56) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+1] << 48) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+2] << 40) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+3] << 32) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+4] << 24) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+5] << 16) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+6] << 8) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7]); - } - /* Avoid unlikely, but theoretically possible overflow */ - if (*capacity > (ULLONG_MAX / fileTypeInfo[i].sizeMultiplier)) - continue; - *capacity *= fileTypeInfo[i].sizeMultiplier; - } - - if (fileTypeInfo[i].qcowCryptOffset != -1) { - int crypt_format; - - crypt_format = (head[fileTypeInfo[i].qcowCryptOffset] << 24) | - (head[fileTypeInfo[i].qcowCryptOffset+1] << 16) | - (head[fileTypeInfo[i].qcowCryptOffset+2] << 8) | - head[fileTypeInfo[i].qcowCryptOffset+3]; - encrypted_qcow = crypt_format != 0; - } - - /* Validation passed, we know the file format now */ - target->format = fileTypeInfo[i].type; - if (fileTypeInfo[i].getBackingStore != NULL && backingStore) { - char *base; - - switch (fileTypeInfo[i].getBackingStore(conn, &base, head, len)) { - case BACKING_STORE_OK: - break; - - case BACKING_STORE_INVALID: - continue; - - case BACKING_STORE_ERROR: - return -1; - } - if (base != NULL) { - *backingStore - = absolutePathFromBaseFile(target->path, base); - VIR_FREE(base); - if (*backingStore == NULL) { - virReportOOMError(conn); - return -1; - } - } - } - if (encryption != NULL && encrypted_qcow) { - virStorageEncryptionPtr enc; - - if (VIR_ALLOC(enc) < 0) { - virReportOOMError(conn); - if (backingStore) - VIR_FREE(*backingStore); - return -1; - } - enc->format = VIR_STORAGE_ENCRYPTION_FORMAT_QCOW; - *encryption = enc; - /* XXX ideally we'd fill in secret UUID here - * but we cannot guarentee 'conn' is non-NULL - * at this point in time :-( So we only fill - * in secrets when someone first queries a vol - */ - } - return 0; - } - - /* No magic, so check file extension */ - for (i = 0 ; i < ARRAY_CARDINALITY(fileTypeInfo) ; i++) { - if (fileTypeInfo[i].extension == NULL) - continue; - - if (!virFileHasSuffix(target->path, fileTypeInfo[i].extension)) - continue; - - target->format = fileTypeInfo[i].type; - return 0; - } - - /* All fails, so call it a raw file */ - target->format = VIR_STORAGE_FILE_RAW; - return 0; -} - -#if WITH_STORAGE_FS - #include <mntent.h> struct _virNetfsDiscoverState { @@ -901,12 +492,14 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn, if ((vol->key = strdup(vol->target.path)) == NULL) goto no_memory; - if ((ret = virStorageBackendProbeTarget(conn, - &vol->target, - &backingStore, - &vol->allocation, - &vol->capacity, - &vol->target.encryption) < 0)) { + if ((ret = virStorageFileProbeHeader(conn, + vol->target.path, + &vol->target.format, + &backingStore, + &vol->target.encryption, + &vol->target.perms, + &vol->allocation, + &vol->capacity) < 0)) { if (ret == -1) goto cleanup; else { @@ -942,10 +535,12 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn, } else { vol->backingStore.path = backingStore; - if ((ret = virStorageBackendProbeTarget(conn, - &vol->backingStore, - NULL, NULL, NULL, - NULL)) < 0) { + if ((ret = virStorageFileProbeHeader(conn, + vol->backingStore.path, + &vol->backingStore.format, + NULL, NULL, + &vol->backingStore.perms, + NULL, NULL)) < 0) { if (ret == -1) goto cleanup; else { diff --git a/src/util/storage_file.c b/src/util/storage_file.c index c1eec9b..70ffac4 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -51,6 +51,415 @@ VIR_ENUM_IMPL(virStorageFileFormat, #define DEV_BSIZE 512 #endif +enum lv_endian { + LV_LITTLE_ENDIAN = 1, /* 1234 */ + LV_BIG_ENDIAN /* 4321 */ +}; + +enum { + BACKING_STORE_OK, + BACKING_STORE_INVALID, + BACKING_STORE_ERROR, +}; + +/* Either 'magic' or 'extension' *must* be provided */ +struct FileTypeInfo { + int type; /* One of the constants above */ + const char *magic; /* Optional string of file magic + * to check at head of file */ + const char *extension; /* Optional file extension to check */ + enum lv_endian endian; /* Endianness of file format */ + int versionOffset; /* Byte offset from start of file + * where we find version number, + * -1 to skip version test */ + int versionNumber; /* Version number to validate */ + int sizeOffset; /* Byte offset from start of file + * where we find capacity info, + * -1 to use st_size as capacity */ + int sizeBytes; /* Number of bytes for size field */ + int sizeMultiplier; /* A scaling factor if size is not in bytes */ + /* Store a COW base image path (possibly relative), + * or NULL if there is no COW base image, to RES; + * return BACKING_STORE_* */ + int qcowCryptOffset; /* Byte offset from start of file + * where to find encryption mode, + * -1 if encryption is not used */ + int (*getBackingStore)(virConnectPtr conn, char **res, + const unsigned char *buf, size_t buf_size); +}; + +static int cowGetBackingStore(virConnectPtr, char **, + const unsigned char *, size_t); +static int qcowXGetBackingStore(virConnectPtr, char **, + const unsigned char *, size_t); +static int vmdk4GetBackingStore(virConnectPtr, char **, + const unsigned char *, size_t); + +static struct FileTypeInfo const fileTypeInfo[] = { + /* Bochs */ + /* XXX Untested + { VIR_STORAGE_FILE_BOCHS, "Bochs Virtual HD Image", NULL, + LV_LITTLE_ENDIAN, 64, 0x20000, + 32+16+16+4+4+4+4+4, 8, 1, -1, NULL },*/ + /* CLoop */ + /* XXX Untested + { VIR_STORAGE_VOL_CLOOP, "#!/bin/sh\n#V2.0 Format\nmodprobe cloop file=$0 && mount -r -t iso9660 /dev/cloop $1\n", NULL, + LV_LITTLE_ENDIAN, -1, 0, + -1, 0, 0, -1, NULL }, */ + /* Cow */ + { VIR_STORAGE_FILE_COW, "OOOM", NULL, + LV_BIG_ENDIAN, 4, 2, + 4+4+1024+4, 8, 1, -1, cowGetBackingStore }, + /* DMG */ + /* XXX QEMU says there's no magic for dmg, but we should check... */ + { VIR_STORAGE_FILE_DMG, NULL, ".dmg", + 0, -1, 0, + -1, 0, 0, -1, NULL }, + /* XXX there's probably some magic for iso we can validate too... */ + { VIR_STORAGE_FILE_ISO, NULL, ".iso", + 0, -1, 0, + -1, 0, 0, -1, NULL }, + /* Parallels */ + /* XXX Untested + { VIR_STORAGE_FILE_PARALLELS, "WithoutFreeSpace", NULL, + LV_LITTLE_ENDIAN, 16, 2, + 16+4+4+4+4, 4, 512, -1, NULL }, + */ + /* QCow */ + { VIR_STORAGE_FILE_QCOW, "QFI", NULL, + LV_BIG_ENDIAN, 4, 1, + 4+4+8+4+4, 8, 1, 4+4+8+4+4+8+1+1+2, qcowXGetBackingStore }, + /* QCow 2 */ + { VIR_STORAGE_FILE_QCOW2, "QFI", NULL, + LV_BIG_ENDIAN, 4, 2, + 4+4+8+4+4, 8, 1, 4+4+8+4+4+8, qcowXGetBackingStore }, + /* VMDK 3 */ + /* XXX Untested + { VIR_STORAGE_FILE_VMDK, "COWD", NULL, + LV_LITTLE_ENDIAN, 4, 1, + 4+4+4, 4, 512, -1, NULL }, + */ + /* VMDK 4 */ + { VIR_STORAGE_FILE_VMDK, "KDMV", NULL, + LV_LITTLE_ENDIAN, 4, 1, + 4+4+4, 8, 512, -1, vmdk4GetBackingStore }, + /* Connectix / VirtualPC */ + /* XXX Untested + { VIR_STORAGE_FILE_VPC, "conectix", NULL, + LV_BIG_ENDIAN, -1, 0, + -1, 0, 0, -1, NULL}, + */ +}; + +static int +cowGetBackingStore(virConnectPtr conn, + char **res, + const unsigned char *buf, + size_t buf_size) +{ +#define COW_FILENAME_MAXLEN 1024 + *res = NULL; + if (buf_size < 4+4+ COW_FILENAME_MAXLEN) + return BACKING_STORE_INVALID; + if (buf[4+4] == '\0') /* cow_header_v2.backing_file[0] */ + return BACKING_STORE_OK; + + *res = strndup ((const char*)buf + 4+4, COW_FILENAME_MAXLEN); + if (*res == NULL) { + virReportOOMError(conn); + return BACKING_STORE_ERROR; + } + return BACKING_STORE_OK; +} + +static int +qcowXGetBackingStore(virConnectPtr conn, + char **res, + const unsigned char *buf, + size_t buf_size) +{ + unsigned long long offset; + unsigned long size; + + *res = NULL; + if (buf_size < 4+4+8+4) + return BACKING_STORE_INVALID; + offset = (((unsigned long long)buf[4+4] << 56) + | ((unsigned long long)buf[4+4+1] << 48) + | ((unsigned long long)buf[4+4+2] << 40) + | ((unsigned long long)buf[4+4+3] << 32) + | ((unsigned long long)buf[4+4+4] << 24) + | ((unsigned long long)buf[4+4+5] << 16) + | ((unsigned long long)buf[4+4+6] << 8) + | buf[4+4+7]); /* QCowHeader.backing_file_offset */ + if (offset > buf_size) + return BACKING_STORE_INVALID; + size = ((buf[4+4+8] << 24) + | (buf[4+4+8+1] << 16) + | (buf[4+4+8+2] << 8) + | buf[4+4+8+3]); /* QCowHeader.backing_file_size */ + if (size == 0) + return BACKING_STORE_OK; + if (offset + size > buf_size || offset + size < offset) + return BACKING_STORE_INVALID; + if (size + 1 == 0) + return BACKING_STORE_INVALID; + if (VIR_ALLOC_N(*res, size + 1) < 0) { + virReportOOMError(conn); + return BACKING_STORE_ERROR; + } + memcpy(*res, buf + offset, size); + (*res)[size] = '\0'; + return BACKING_STORE_OK; +} + +static int +vmdk4GetBackingStore(virConnectPtr conn, + char **res, + const unsigned char *buf, + size_t buf_size) +{ + static const char prefix[] = "parentFileNameHint=\""; + + char desc[20*512 + 1], *start, *end; + size_t len; + + *res = NULL; + + if (buf_size <= 0x200) + return BACKING_STORE_INVALID; + len = buf_size - 0x200; + if (len > sizeof(desc) - 1) + len = sizeof(desc) - 1; + memcpy(desc, buf + 0x200, len); + desc[len] = '\0'; + start = strstr(desc, prefix); + if (start == NULL) + return BACKING_STORE_OK; + start += strlen(prefix); + end = strchr(start, '"'); + if (end == NULL) + return BACKING_STORE_INVALID; + if (end == start) + return BACKING_STORE_OK; + *end = '\0'; + *res = strdup(start); + if (*res == NULL) { + virReportOOMError(conn); + return BACKING_STORE_ERROR; + } + return BACKING_STORE_OK; +} + +/** + * Return an absolute path corresponding to PATH, which is absolute or relative + * to the directory containing BASE_FILE, or NULL on error + */ +static char * +absolutePathFromBaseFile(const char *base_file, const char *path) +{ + size_t base_size, path_size; + char *res, *p; + + if (*path == '/') + return strdup(path); + + base_size = strlen(base_file) + 1; + path_size = strlen(path) + 1; + if (VIR_ALLOC_N(res, base_size - 1 + path_size) < 0) + return NULL; + memcpy(res, base_file, base_size); + p = strrchr(res, '/'); + if (p != NULL) + p++; + else + p = res; + memcpy(p, path, path_size); + if (VIR_REALLOC_N(res, (p + path_size) - res) < 0) { + /* Ignore failure */ + } + return res; +} + +/** + * Probe the header of a file to determine what type of disk image + * it is, and info about its capacity if available. + */ +int +virStorageFileProbeHeader(virConnectPtr conn, + const char *path, + int *format, + char **backingStore, + virStorageEncryptionPtr *encryption, + virStorageFilePermsPtr perms, + unsigned long long *allocation, + unsigned long long *capacity) +{ + int fd; + unsigned char head[20*512]; /* vmdk4GetBackingStore needs this much. */ + int len, i, ret; + + if (format) /* If all else fails, call it a raw file */ + *format = VIR_STORAGE_FILE_RAW; + if (backingStore) + *backingStore = NULL; + if (encryption) + *encryption = NULL; + + if ((fd = open(path, O_RDONLY)) < 0) { + virReportSystemError(conn, errno, + _("cannot open volume '%s'"), path); + return -1; + } + + if ((ret = virStorageFileGetInfoFromFD(conn, path, fd, perms, + allocation, capacity)) < 0) { + close(fd); + return ret; /* Take care to propagate ret, it is not always -1 */ + } + + if ((len = read(fd, head, sizeof(head))) < 0) { + virReportSystemError(conn, errno, + _("cannot read header '%s'"), path); + close(fd); + return -1; + } + + close(fd); + + /* First check file magic */ + for (i = 0 ; i < ARRAY_CARDINALITY(fileTypeInfo) ; i++) { + int mlen; + bool encrypted_qcow = false; + + if (fileTypeInfo[i].magic == NULL) + continue; + + /* Validate magic data */ + mlen = strlen(fileTypeInfo[i].magic); + if (mlen > len) + continue; + if (memcmp(head, fileTypeInfo[i].magic, mlen) != 0) + continue; + + /* Validate version number info */ + if (fileTypeInfo[i].versionNumber != -1) { + int version; + + if (fileTypeInfo[i].endian == LV_LITTLE_ENDIAN) { + version = (head[fileTypeInfo[i].versionOffset+3] << 24) | + (head[fileTypeInfo[i].versionOffset+2] << 16) | + (head[fileTypeInfo[i].versionOffset+1] << 8) | + head[fileTypeInfo[i].versionOffset]; + } else { + version = (head[fileTypeInfo[i].versionOffset] << 24) | + (head[fileTypeInfo[i].versionOffset+1] << 16) | + (head[fileTypeInfo[i].versionOffset+2] << 8) | + head[fileTypeInfo[i].versionOffset+3]; + } + if (version != fileTypeInfo[i].versionNumber) + continue; + } + + /* Optionally extract capacity from file */ + if (fileTypeInfo[i].sizeOffset != -1 && capacity) { + if (fileTypeInfo[i].endian == LV_LITTLE_ENDIAN) { + *capacity = + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7] << 56) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+6] << 48) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+5] << 40) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+4] << 32) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+3] << 24) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+2] << 16) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+1] << 8) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset]); + } else { + *capacity = + ((unsigned long long)head[fileTypeInfo[i].sizeOffset] << 56) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+1] << 48) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+2] << 40) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+3] << 32) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+4] << 24) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+5] << 16) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+6] << 8) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7]); + } + /* Avoid unlikely, but theoretically possible overflow */ + if (*capacity > (ULLONG_MAX / fileTypeInfo[i].sizeMultiplier)) + continue; + *capacity *= fileTypeInfo[i].sizeMultiplier; + } + + if (fileTypeInfo[i].qcowCryptOffset != -1) { + int crypt_format; + + crypt_format = (head[fileTypeInfo[i].qcowCryptOffset] << 24) | + (head[fileTypeInfo[i].qcowCryptOffset+1] << 16) | + (head[fileTypeInfo[i].qcowCryptOffset+2] << 8) | + head[fileTypeInfo[i].qcowCryptOffset+3]; + encrypted_qcow = crypt_format != 0; + } + + /* Validation passed, we know the file format now */ + if (format) + *format = fileTypeInfo[i].type; + if (fileTypeInfo[i].getBackingStore != NULL && backingStore) { + char *base; + + switch (fileTypeInfo[i].getBackingStore(conn, &base, head, len)) { + case BACKING_STORE_OK: + break; + + case BACKING_STORE_INVALID: + continue; + + case BACKING_STORE_ERROR: + return -1; + } + if (base != NULL) { + *backingStore = absolutePathFromBaseFile(path, base); + VIR_FREE(base); + if (*backingStore == NULL) { + virReportOOMError(conn); + return -1; + } + } + } + if (encryption != NULL && encrypted_qcow) { + if (VIR_ALLOC(*encryption) < 0) { + virReportOOMError(conn); + if (backingStore) + VIR_FREE(*backingStore); + return -1; + } + (*encryption)->format = VIR_STORAGE_ENCRYPTION_FORMAT_QCOW; + /* XXX ideally we'd fill in secret UUID here + * but we cannot guarentee 'conn' is non-NULL + * at this point in time :-( So we only fill + * in secrets when someone first queries a vol + */ + } + return 0; + } + + if (!format) + return 0; + + /* No magic, so check file extension */ + for (i = 0 ; i < ARRAY_CARDINALITY(fileTypeInfo) ; i++) { + if (fileTypeInfo[i].extension == NULL) + continue; + + if (!virFileHasSuffix(path, fileTypeInfo[i].extension)) + continue; + + *format = fileTypeInfo[i].type; + return 0; + } + + return 0; +} + /* * virStorageFileGetInfoFromFD: * @conn: connection to report errors on diff --git a/src/util/storage_file.h b/src/util/storage_file.h index efacc65..908ca3e 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -25,6 +25,7 @@ #define __VIR_STORAGE_FILE_H__ #include "util.h" +#include "storage_encryption.h" enum virStorageFileFormat { VIR_STORAGE_FILE_RAW = 0, @@ -52,6 +53,15 @@ struct _virStorageFilePerms { char *label; }; +int virStorageFileProbeHeader(virConnectPtr conn, + const char *path, + int *format, + char **backingStore, + virStorageEncryptionPtr *encryption, + virStorageFilePermsPtr perms, + unsigned long long *allocation, + unsigned long long *capacity); + int virStorageFileGetInfo(virConnectPtr conn, const char *path, virStorageFilePermsPtr perms, -- 1.6.2.5

On Fri, Sep 25, 2009 at 02:27:32PM +0100, Mark McLoughlin wrote:
+int virStorageFileProbeHeader(virConnectPtr conn, + const char *path, + int *format, + char **backingStore, + virStorageEncryptionPtr *encryption, + virStorageFilePermsPtr perms, + unsigned long long *allocation, + unsigned long long *capacity); +
This function prototype is the root cause of most of my objections earlier in the series. The way it has evolved over time since I first wrote it has resulted in sub-optimal separation of responsibilities. In particular I don't think this method should be responsible for populating the virStorageEncryptionPtr or virStorageFilePermsPtr structures. It should focus itself to only be concerned with extracting metadata from the file header. The callers can later populate the higher level structures like virStorageFilePermsPtr & virStorageEncryptionPtr as they see fit. For an API in the util/ library I think it'd be reasonable to aim for an simpler API such as int virStorageFileGetMetadata(virConnectPtr conn, const char *path, int *format, char **backingStore, boolean *encrypted unsigned long long *allocation, unsigned long long *capacity); The backingStore, encrypted, allocation & capacity fields would only be filed in for format != VIR_STORAGE_FILE_RAW. There's probably enough return parameters here to justify inventing a small (caller allocated) struct for the returned metadata, such as typedef struct _virStorageFileMetadata { int format; char *backingStore; boolean encrypted; unsigned long long allocation; unsigned long long capacty; } virStorageFileMetadata; So callers would do virStorageFileMetadata meta; memset(&meta, 0, sizeof meta); virStorageFileGetMetadata(conn, path, &meta); The original virStorageBackendProbeTarget() method in storage_backend_fs.c could then be written in terms of a combination of calls to the virStorageFileGetMetadata() and virStorageBackendUpdateVolTargetInfo() methods. This would avoid the need to move most of the code in this series. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Rename virStorageVolFormatFileSystem to virStorageFileFormat and move to src/util/storage_file.[ch] * src/Makefile.am: add src/util/storage_file.[ch] * src/conf/storage_conf.[ch]: move enum from here ... * src/util/storage_file.[ch]: .. to here * src/libvirt_private.syms: update To/FromString exports * src/storage/storage_backend.c, src/storage/storage_backend_fs.c, src/vbox/vbox_tmpl.c: update for above changes --- src/Makefile.am | 1 + src/conf/storage_conf.c | 25 ++++++++------------ src/conf/storage_conf.h | 17 -------------- src/libvirt_private.syms | 5 ++- src/storage/storage_backend.c | 17 +++++++------ src/storage/storage_backend_fs.c | 33 ++++++++++++++------------- src/util/storage_file.c | 31 +++++++++++++++++++++++++ src/util/storage_file.h | 46 ++++++++++++++++++++++++++++++++++++++ src/vbox/vbox_tmpl.c | 15 ++++++----- 9 files changed, 125 insertions(+), 65 deletions(-) create mode 100644 src/util/storage_file.c create mode 100644 src/util/storage_file.h diff --git a/src/Makefile.am b/src/Makefile.am index 9cbec47..47bc39b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -49,6 +49,7 @@ UTIL_SOURCES = \ util/hostusb.c util/hostusb.h \ util/qparams.c util/qparams.h \ util/stats_linux.c util/stats_linux.h \ + util/storage_file.c util/storage_file.h \ util/threads.c util/threads.h \ util/threads-pthread.h \ util/threads-win32.h \ diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index cb063cc..788de15 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -36,6 +36,7 @@ #include "virterror_internal.h" #include "datatypes.h" #include "storage_conf.h" +#include "storage_file.h" #include "xml.h" #include "uuid.h" @@ -82,12 +83,6 @@ VIR_ENUM_IMPL(virStorageVolFormatDisk, "linux-lvm", "linux-raid", "extended") -VIR_ENUM_IMPL(virStorageVolFormatFileSystem, - VIR_STORAGE_VOL_FILE_LAST, - "raw", "dir", "bochs", - "cloop", "cow", "dmg", "iso", - "qcow", "qcow2", "vmdk", "vpc") - VIR_ENUM_IMPL(virStoragePartedFsType, VIR_STORAGE_PARTED_FS_TYPE_LAST, "ext2", "ext2", "fat16", @@ -150,9 +145,9 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { }, { .poolType = VIR_STORAGE_POOL_DIR, .volOptions = { - .defaultFormat = VIR_STORAGE_VOL_FILE_RAW, - .formatFromString = virStorageVolFormatFileSystemTypeFromString, - .formatToString = virStorageVolFormatFileSystemTypeToString, + .defaultFormat = VIR_STORAGE_FILE_RAW, + .formatFromString = virStorageFileFormatTypeFromString, + .formatToString = virStorageFileFormatTypeToString, }, }, { .poolType = VIR_STORAGE_POOL_FS, @@ -162,9 +157,9 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { .formatToString = virStoragePoolFormatFileSystemTypeToString, }, .volOptions = { - .defaultFormat = VIR_STORAGE_VOL_FILE_RAW, - .formatFromString = virStorageVolFormatFileSystemTypeFromString, - .formatToString = virStorageVolFormatFileSystemTypeToString, + .defaultFormat = VIR_STORAGE_FILE_RAW, + .formatFromString = virStorageFileFormatTypeFromString, + .formatToString = virStorageFileFormatTypeToString, }, }, { .poolType = VIR_STORAGE_POOL_NETFS, @@ -176,9 +171,9 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { .formatToString = virStoragePoolFormatFileSystemNetTypeToString, }, .volOptions = { - .defaultFormat = VIR_STORAGE_VOL_FILE_RAW, - .formatFromString = virStorageVolFormatFileSystemTypeFromString, - .formatToString = virStorageVolFormatFileSystemTypeToString, + .defaultFormat = VIR_STORAGE_FILE_RAW, + .formatFromString = virStorageFileFormatTypeFromString, + .formatToString = virStorageFileFormatTypeToString, }, }, { .poolType = VIR_STORAGE_POOL_ISCSI, diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 421d305..9fedb12 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -429,23 +429,6 @@ enum virStoragePoolFormatLogical { }; VIR_ENUM_DECL(virStoragePoolFormatLogical) - -enum virStorageVolFormatFileSystem { - VIR_STORAGE_VOL_FILE_RAW = 0, - VIR_STORAGE_VOL_FILE_DIR, - VIR_STORAGE_VOL_FILE_BOCHS, - VIR_STORAGE_VOL_FILE_CLOOP, - VIR_STORAGE_VOL_FILE_COW, - VIR_STORAGE_VOL_FILE_DMG, - VIR_STORAGE_VOL_FILE_ISO, - VIR_STORAGE_VOL_FILE_QCOW, - VIR_STORAGE_VOL_FILE_QCOW2, - VIR_STORAGE_VOL_FILE_VMDK, - VIR_STORAGE_VOL_FILE_VPC, - VIR_STORAGE_VOL_FILE_LAST, -}; -VIR_ENUM_DECL(virStorageVolFormatFileSystem) - /* * XXX these are basically partition types. * diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a6668f3..ddab21d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -378,8 +378,6 @@ virStorageVolDefParseNode; virStoragePoolFormatDiskTypeToString; virStoragePoolFormatFileSystemTypeToString; virStoragePoolFormatFileSystemNetTypeToString; -virStorageVolFormatFileSystemTypeToString; -virStorageVolFormatFileSystemTypeFromString; virStoragePoolTypeFromString; virStoragePartedFsTypeTypeToString; virStoragePoolObjLock; @@ -392,6 +390,9 @@ virStorageEncryptionParseNode; virStorageEncryptionFormat; virStorageGenerateQcowPassphrase; +# storage_file.h +virStorageFileFormatTypeToString; +virStorageFileFormatTypeFromString; # threads.h virMutexInit; diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index b3c9b0e..18a69be 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -50,6 +50,7 @@ #include "internal.h" #include "secret_conf.h" #include "uuid.h" +#include "storage_file.h" #include "storage_backend.h" #include "logging.h" @@ -461,16 +462,16 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, char *create_tool; short use_kvmimg; - const char *type = virStorageVolFormatFileSystemTypeToString(vol->target.format); + const char *type = virStorageFileFormatTypeToString(vol->target.format); const char *backingType = vol->backingStore.path ? - virStorageVolFormatFileSystemTypeToString(vol->backingStore.format) : NULL; + virStorageFileFormatTypeToString(vol->backingStore.format) : NULL; const char *inputBackingPath = (inputvol ? inputvol->backingStore.path : NULL); const char *inputPath = inputvol ? inputvol->target.path : NULL; /* Treat input block devices as 'raw' format */ const char *inputType = inputPath ? - virStorageVolFormatFileSystemTypeToString(inputvol->type == VIR_STORAGE_VOL_BLOCK ? VIR_STORAGE_VOL_FILE_RAW : inputvol->target.format) : + virStorageFileFormatTypeToString(inputvol->type == VIR_STORAGE_VOL_BLOCK ? VIR_STORAGE_FILE_RAW : inputvol->target.format) : NULL; const char **imgargv; @@ -551,8 +552,8 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, if (vol->target.encryption != NULL) { virStorageEncryptionPtr enc; - if (vol->target.format != VIR_STORAGE_VOL_FILE_QCOW && - vol->target.format != VIR_STORAGE_VOL_FILE_QCOW2) { + if (vol->target.format != VIR_STORAGE_FILE_QCOW && + vol->target.format != VIR_STORAGE_FILE_QCOW2) { virStorageReportError(conn, VIR_ERR_NO_SUPPORT, _("qcow volume encryption unsupported with " "volume format %s"), type); @@ -643,7 +644,7 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn, return -1; } - if (vol->target.format != VIR_STORAGE_VOL_FILE_QCOW2) { + if (vol->target.format != VIR_STORAGE_FILE_QCOW2) { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, _("unsupported storage vol type %d"), vol->target.format); @@ -734,9 +735,9 @@ virStorageBackendGetBuildVolFromFunction(virConnectPtr conn, * tool for converting */ if ((vol->type == VIR_STORAGE_VOL_FILE && - vol->target.format != VIR_STORAGE_VOL_FILE_RAW) || + vol->target.format != VIR_STORAGE_FILE_RAW) || (inputvol->type == VIR_STORAGE_VOL_FILE && - inputvol->target.format != VIR_STORAGE_VOL_FILE_RAW)) { + inputvol->target.format != VIR_STORAGE_FILE_RAW)) { if ((tool_type = virStorageBackendFindFSImageTool(NULL)) < 0) { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 01cb171..e7a3ca9 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -41,6 +41,7 @@ #include "virterror_internal.h" #include "storage_backend_fs.h" #include "storage_conf.h" +#include "storage_file.h" #include "util.h" #include "memory.h" #include "xml.h" @@ -91,7 +92,7 @@ struct FileTypeInfo { struct FileTypeInfo const fileTypeInfo[] = { /* Bochs */ /* XXX Untested - { VIR_STORAGE_VOL_FILE_BOCHS, "Bochs Virtual HD Image", NULL, + { VIR_STORAGE_FILE_BOCHS, "Bochs Virtual HD Image", NULL, LV_LITTLE_ENDIAN, 64, 0x20000, 32+16+16+4+4+4+4+4, 8, 1, -1, NULL },*/ /* CLoop */ @@ -100,45 +101,45 @@ struct FileTypeInfo const fileTypeInfo[] = { LV_LITTLE_ENDIAN, -1, 0, -1, 0, 0, -1, NULL }, */ /* Cow */ - { VIR_STORAGE_VOL_FILE_COW, "OOOM", NULL, + { VIR_STORAGE_FILE_COW, "OOOM", NULL, LV_BIG_ENDIAN, 4, 2, 4+4+1024+4, 8, 1, -1, cowGetBackingStore }, /* DMG */ /* XXX QEMU says there's no magic for dmg, but we should check... */ - { VIR_STORAGE_VOL_FILE_DMG, NULL, ".dmg", + { VIR_STORAGE_FILE_DMG, NULL, ".dmg", 0, -1, 0, -1, 0, 0, -1, NULL }, /* XXX there's probably some magic for iso we can validate too... */ - { VIR_STORAGE_VOL_FILE_ISO, NULL, ".iso", + { VIR_STORAGE_FILE_ISO, NULL, ".iso", 0, -1, 0, -1, 0, 0, -1, NULL }, /* Parallels */ /* XXX Untested - { VIR_STORAGE_VOL_FILE_PARALLELS, "WithoutFreeSpace", NULL, + { VIR_STORAGE_FILE_PARALLELS, "WithoutFreeSpace", NULL, LV_LITTLE_ENDIAN, 16, 2, 16+4+4+4+4, 4, 512, -1, NULL }, */ /* QCow */ - { VIR_STORAGE_VOL_FILE_QCOW, "QFI", NULL, + { VIR_STORAGE_FILE_QCOW, "QFI", NULL, LV_BIG_ENDIAN, 4, 1, 4+4+8+4+4, 8, 1, 4+4+8+4+4+8+1+1+2, qcowXGetBackingStore }, /* QCow 2 */ - { VIR_STORAGE_VOL_FILE_QCOW2, "QFI", NULL, + { VIR_STORAGE_FILE_QCOW2, "QFI", NULL, LV_BIG_ENDIAN, 4, 2, 4+4+8+4+4, 8, 1, 4+4+8+4+4+8, qcowXGetBackingStore }, /* VMDK 3 */ /* XXX Untested - { VIR_STORAGE_VOL_FILE_VMDK, "COWD", NULL, + { VIR_STORAGE_FILE_VMDK, "COWD", NULL, LV_LITTLE_ENDIAN, 4, 1, 4+4+4, 4, 512, -1, NULL }, */ /* VMDK 4 */ - { VIR_STORAGE_VOL_FILE_VMDK, "KDMV", NULL, + { VIR_STORAGE_FILE_VMDK, "KDMV", NULL, LV_LITTLE_ENDIAN, 4, 1, 4+4+4, 8, 512, -1, vmdk4GetBackingStore }, /* Connectix / VirtualPC */ /* XXX Untested - { VIR_STORAGE_VOL_FILE_VPC, "conectix", NULL, + { VIR_STORAGE_FILE_VPC, "conectix", NULL, LV_BIG_ENDIAN, -1, 0, -1, 0, 0, -1, NULL}, */ @@ -452,7 +453,7 @@ static int virStorageBackendProbeTarget(virConnectPtr conn, } /* All fails, so call it a raw file */ - target->format = VIR_STORAGE_VOL_FILE_RAW; + target->format = VIR_STORAGE_FILE_RAW; return 0; } @@ -891,7 +892,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn, goto no_memory; vol->type = VIR_STORAGE_VOL_FILE; - vol->target.format = VIR_STORAGE_VOL_FILE_RAW; /* Real value is filled in during probe */ + vol->target.format = VIR_STORAGE_FILE_RAW; /* Real value is filled in during probe */ if (virAsprintf(&vol->target.path, "%s/%s", pool->def->target.path, vol->name) == -1) @@ -918,7 +919,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn, } if (backingStore != NULL) { - if (vol->target.format == VIR_STORAGE_VOL_FILE_QCOW2 && + if (vol->target.format == VIR_STORAGE_FILE_QCOW2 && STRPREFIX("fmt:", backingStore)) { char *fmtstr = backingStore + 4; char *path = strchr(fmtstr, ':'); @@ -927,7 +928,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn, } else { *path = '\0'; if ((vol->backingStore.format = - virStorageVolFormatFileSystemTypeFromString(fmtstr)) < 0) { + virStorageFileFormatTypeFromString(fmtstr)) < 0) { VIR_FREE(backingStore); } else { memmove(backingStore, path, strlen(path) + 1); @@ -1121,9 +1122,9 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn, inputvol); if (!create_func) return -1; - } else if (vol->target.format == VIR_STORAGE_VOL_FILE_RAW) { + } else if (vol->target.format == VIR_STORAGE_FILE_RAW) { create_func = virStorageBackendCreateRaw; - } else if (vol->target.format == VIR_STORAGE_VOL_FILE_DIR) { + } else if (vol->target.format == VIR_STORAGE_FILE_DIR) { create_func = createFileDir; } else if ((tool_type = virStorageBackendFindFSImageTool(NULL)) != -1) { create_func = virStorageBackendFSImageToolTypeToFunc(conn, tool_type); diff --git a/src/util/storage_file.c b/src/util/storage_file.c new file mode 100644 index 0000000..e66ec8a --- /dev/null +++ b/src/util/storage_file.c @@ -0,0 +1,31 @@ +/* + * storage_file.c: file utility functions for FS storage backend + * + * Copyright (C) 2007-2009 Red Hat, Inc. + * Copyright (C) 2007-2008 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#include <config.h> +#include "storage_file.h" + +VIR_ENUM_IMPL(virStorageFileFormat, + VIR_STORAGE_FILE_LAST, + "raw", "dir", "bochs", + "cloop", "cow", "dmg", "iso", + "qcow", "qcow2", "vmdk", "vpc") diff --git a/src/util/storage_file.h b/src/util/storage_file.h new file mode 100644 index 0000000..7bccbe4 --- /dev/null +++ b/src/util/storage_file.h @@ -0,0 +1,46 @@ +/* + * storage_file.c: file utility functions for FS storage backend + * + * Copyright (C) 2007-2009 Red Hat, Inc. + * Copyright (C) 2007-2008 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#ifndef __VIR_STORAGE_FILE_H__ +#define __VIR_STORAGE_FILE_H__ + +#include "util.h" + +enum virStorageFileFormat { + VIR_STORAGE_FILE_RAW = 0, + VIR_STORAGE_FILE_DIR, + VIR_STORAGE_FILE_BOCHS, + VIR_STORAGE_FILE_CLOOP, + VIR_STORAGE_FILE_COW, + VIR_STORAGE_FILE_DMG, + VIR_STORAGE_FILE_ISO, + VIR_STORAGE_FILE_QCOW, + VIR_STORAGE_FILE_QCOW2, + VIR_STORAGE_FILE_VMDK, + VIR_STORAGE_FILE_VPC, + VIR_STORAGE_FILE_LAST, +}; + +VIR_ENUM_DECL(virStorageFileFormat); + +#endif /* __VIR_STORAGE_FILE_H__ */ diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index c02b18c..ecad2a1 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -45,6 +45,7 @@ #include "virterror_internal.h" #include "domain_event.h" #include "storage_conf.h" +#include "storage_file.h" #include "uuid.h" #include "event.h" #include "memory.h" @@ -5980,14 +5981,14 @@ static virStorageVolPtr vboxStorageVolCreateXML(virStoragePoolPtr pool, /* TODO: for now only the vmdk, vpc and vdi type harddisk * variants can be created, also since there is no vdi - * type in enum virStorageVolFormatFileSystem {} the default + * type in enum virStorageFileFormat {} the default * will be to create vdi if nothing is specified in * def->target.format */ - if (def->target.format == VIR_STORAGE_VOL_FILE_VMDK) { + if (def->target.format == VIR_STORAGE_FILE_VMDK) { data->pFuncs->pfnUtf8ToUtf16("VMDK", &hddFormatUtf16); - } else if (def->target.format == VIR_STORAGE_VOL_FILE_VPC) { + } else if (def->target.format == VIR_STORAGE_FILE_VPC) { data->pFuncs->pfnUtf8ToUtf16("VHD", &hddFormatUtf16); } else { data->pFuncs->pfnUtf8ToUtf16("VDI", &hddFormatUtf16); @@ -6302,13 +6303,13 @@ static char *vboxStorageVolGetXMLDesc(virStorageVolPtr vol, unsigned int flags A DEBUG("Storage Volume Format: %s", hddFormatUtf8); if (STRCASEEQ("vmdk", hddFormatUtf8)) - def.target.format = VIR_STORAGE_VOL_FILE_VMDK; + def.target.format = VIR_STORAGE_FILE_VMDK; else if (STRCASEEQ("vhd", hddFormatUtf8)) - def.target.format = VIR_STORAGE_VOL_FILE_VPC; + def.target.format = VIR_STORAGE_FILE_VPC; else - def.target.format = VIR_STORAGE_VOL_FILE_RAW; + def.target.format = VIR_STORAGE_FILE_RAW; - /* TODO: need to add vdi to enum virStorageVolFormatFileSystem {} + /* TODO: need to add vdi to enum virStorageFileFormat {} * and then add it here */ -- 1.6.2.5

+enum virStorageFileFormat { + VIR_STORAGE_FILE_RAW = 0, + VIR_STORAGE_FILE_DIR, + VIR_STORAGE_FILE_BOCHS, + VIR_STORAGE_FILE_CLOOP, + VIR_STORAGE_FILE_COW, + VIR_STORAGE_FILE_DMG, + VIR_STORAGE_FILE_ISO, + VIR_STORAGE_FILE_QCOW, + VIR_STORAGE_FILE_QCOW2, + VIR_STORAGE_FILE_VMDK, + VIR_STORAGE_FILE_VPC, + VIR_STORAGE_FILE_LAST, +};
Any chance we could add VIR_STORAGE_FILE_VDI (VirtualBox) and VIR_STORAGE_FILE_HDD (Parallels) here as well? Regards, Pritesh

On Tue, Sep 29, 2009 at 11:27:18AM +0200, Pritesh Kothari wrote:
+enum virStorageFileFormat { + VIR_STORAGE_FILE_RAW = 0, + VIR_STORAGE_FILE_DIR, + VIR_STORAGE_FILE_BOCHS, + VIR_STORAGE_FILE_CLOOP, + VIR_STORAGE_FILE_COW, + VIR_STORAGE_FILE_DMG, + VIR_STORAGE_FILE_ISO, + VIR_STORAGE_FILE_QCOW, + VIR_STORAGE_FILE_QCOW2, + VIR_STORAGE_FILE_VMDK, + VIR_STORAGE_FILE_VPC, + VIR_STORAGE_FILE_LAST, +};
Any chance we could add VIR_STORAGE_FILE_VDI (VirtualBox) and VIR_STORAGE_FILE_HDD (Parallels) here as well?
Of course - patches to add more formats are welcome. The formats we have there so far are just the ones I was able to figure out from the qemu-img source. At the minimum level you just need to figure out the magic bytes to identify the format, and the offset into the file where the logical capacity is stored. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Tue, Sep 29, 2009 at 09:56:44AM +0100, Mark McLoughlin wrote:
Rename virStorageVolFormatFileSystem to virStorageFileFormat and move to src/util/storage_file.[ch]
* src/Makefile.am: add src/util/storage_file.[ch]
* src/conf/storage_conf.[ch]: move enum from here ...
* src/util/storage_file.[ch]: .. to here
* src/libvirt_private.syms: update To/FromString exports
* src/storage/storage_backend.c, src/storage/storage_backend_fs.c, src/vbox/vbox_tmpl.c: update for above changes --- src/Makefile.am | 1 + src/conf/storage_conf.c | 25 ++++++++------------ src/conf/storage_conf.h | 17 -------------- src/libvirt_private.syms | 5 ++- src/storage/storage_backend.c | 17 +++++++------ src/storage/storage_backend_fs.c | 33 ++++++++++++++------------- src/util/storage_file.c | 31 +++++++++++++++++++++++++ src/util/storage_file.h | 46 ++++++++++++++++++++++++++++++++++++++ src/vbox/vbox_tmpl.c | 15 ++++++----- 9 files changed, 125 insertions(+), 65 deletions(-) create mode 100644 src/util/storage_file.c create mode 100644 src/util/storage_file.h
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Prepare the code probing a file's format and associated metadata for moving into libvirt_util. * src/storage/storage_backend_fs.c: re-factor the format and metadata probing code in preparation for moving it --- src/storage/storage_backend_fs.c | 148 +++++++++++++++++++++++--------------- 1 files changed, 91 insertions(+), 57 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index e7a3ca9..87c30cd 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -283,49 +283,37 @@ static char *absolutePathFromBaseFile(const char *base_file, const char *path) * Probe the header of a file to determine what type of disk image * it is, and info about its capacity if available. */ -static int virStorageBackendProbeTarget(virConnectPtr conn, - virStorageVolTargetPtr target, - char **backingStore, - unsigned long long *allocation, - unsigned long long *capacity, - virStorageEncryptionPtr *encryption) { - int fd; +static int +virStorageGetMetadataFromFD(virConnectPtr conn, + const char *path, + int fd, + int *format, + bool *encrypted, + char **backingStore, + unsigned long long *capacity) +{ unsigned char head[20*512]; /* vmdk4GetBackingStore needs this much. */ - int len, i, ret; + int len, i; + if (format) /* If all else fails, call it a raw file */ + *format = VIR_STORAGE_FILE_RAW; + if (encrypted) + *encrypted = false; if (backingStore) *backingStore = NULL; - if (encryption) - *encryption = NULL; - - if ((fd = open(target->path, O_RDONLY)) < 0) { - virReportSystemError(conn, errno, - _("cannot open volume '%s'"), - target->path); - return -1; - } - - if ((ret = virStorageBackendUpdateVolTargetInfoFD(conn, target, fd, - allocation, - capacity)) < 0) { - close(fd); - return ret; /* Take care to propagate ret, it is not always -1 */ - } + /* Do not overwrite capacity + * if (capacity) + * *capacity = 0; + */ if ((len = read(fd, head, sizeof(head))) < 0) { - virReportSystemError(conn, errno, - _("cannot read header '%s'"), - target->path); - close(fd); + virReportSystemError(conn, errno, _("cannot read header '%s'"), path); return -1; } - close(fd); - /* First check file magic */ for (i = 0 ; i < ARRAY_CARDINALITY(fileTypeInfo) ; i++) { int mlen; - bool encrypted_qcow = false; if (fileTypeInfo[i].magic == NULL) continue; @@ -385,18 +373,19 @@ static int virStorageBackendProbeTarget(virConnectPtr conn, *capacity *= fileTypeInfo[i].sizeMultiplier; } - if (fileTypeInfo[i].qcowCryptOffset != -1) { + if (fileTypeInfo[i].qcowCryptOffset != -1 && encrypted) { int crypt_format; crypt_format = (head[fileTypeInfo[i].qcowCryptOffset] << 24) | (head[fileTypeInfo[i].qcowCryptOffset+1] << 16) | (head[fileTypeInfo[i].qcowCryptOffset+2] << 8) | head[fileTypeInfo[i].qcowCryptOffset+3]; - encrypted_qcow = crypt_format != 0; + *encrypted = crypt_format != 0; } /* Validation passed, we know the file format now */ - target->format = fileTypeInfo[i].type; + if (format) + *format = fileTypeInfo[i].type; if (fileTypeInfo[i].getBackingStore != NULL && backingStore) { char *base; @@ -411,8 +400,7 @@ static int virStorageBackendProbeTarget(virConnectPtr conn, return -1; } if (base != NULL) { - *backingStore - = absolutePathFromBaseFile(target->path, base); + *backingStore = absolutePathFromBaseFile(path, base); VIR_FREE(base); if (*backingStore == NULL) { virReportOOMError(conn); @@ -420,23 +408,6 @@ static int virStorageBackendProbeTarget(virConnectPtr conn, } } } - if (encryption != NULL && encrypted_qcow) { - virStorageEncryptionPtr enc; - - if (VIR_ALLOC(enc) < 0) { - virReportOOMError(conn); - if (backingStore) - VIR_FREE(*backingStore); - return -1; - } - enc->format = VIR_STORAGE_ENCRYPTION_FORMAT_QCOW; - *encryption = enc; - /* XXX ideally we'd fill in secret UUID here - * but we cannot guarentee 'conn' is non-NULL - * at this point in time :-( So we only fill - * in secrets when someone first queries a vol - */ - } return 0; } @@ -445,15 +416,78 @@ static int virStorageBackendProbeTarget(virConnectPtr conn, if (fileTypeInfo[i].extension == NULL) continue; - if (!virFileHasSuffix(target->path, fileTypeInfo[i].extension)) + if (!virFileHasSuffix(path, fileTypeInfo[i].extension)) continue; - target->format = fileTypeInfo[i].type; + if (format) + *format = fileTypeInfo[i].type; return 0; } - /* All fails, so call it a raw file */ - target->format = VIR_STORAGE_FILE_RAW; + return 0; +} + +static int +virStorageBackendProbeTarget(virConnectPtr conn, + virStorageVolTargetPtr target, + char **backingStore, + unsigned long long *allocation, + unsigned long long *capacity, + virStorageEncryptionPtr *encryption) +{ + int fd, ret; + bool encrypted; + + if (encryption) + *encryption = NULL; + + if ((fd = open(target->path, O_RDONLY)) < 0) { + virReportSystemError(conn, errno, + _("cannot open volume '%s'"), + target->path); + return -1; + } + + if ((ret = virStorageBackendUpdateVolTargetInfoFD(conn, target, fd, + allocation, + capacity)) < 0) { + close(fd); + return ret; /* Take care to propagate ret, it is not always -1 */ + } + + if (virStorageGetMetadataFromFD(conn, target->path, fd, + &target->format, &encrypted, + backingStore, capacity) < 0) { + close(fd); + return -1; + } + + close(fd); + + if (encryption != NULL && encrypted) { + if (VIR_ALLOC(*encryption) < 0) { + virReportOOMError(conn); + if (backingStore) + VIR_FREE(*backingStore); + return -1; + } + + switch (target->format) { + case VIR_STORAGE_FILE_QCOW: + case VIR_STORAGE_FILE_QCOW2: + (*encryption)->format = VIR_STORAGE_ENCRYPTION_FORMAT_QCOW; + break; + default: + break; + } + + /* XXX ideally we'd fill in secret UUID here + * but we cannot guarentee 'conn' is non-NULL + * at this point in time :-( So we only fill + * in secrets when someone first queries a vol + */ + } + return 0; } -- 1.6.2.5

On Tue, Sep 29, 2009 at 09:56:45AM +0100, Mark McLoughlin wrote:
Prepare the code probing a file's format and associated metadata for moving into libvirt_util.
* src/storage/storage_backend_fs.c: re-factor the format and metadata probing code in preparation for moving it --- src/storage/storage_backend_fs.c | 148 +++++++++++++++++++++++--------------- 1 files changed, 91 insertions(+), 57 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index e7a3ca9..87c30cd 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -283,49 +283,37 @@ static char *absolutePathFromBaseFile(const char *base_file, const char *path) * Probe the header of a file to determine what type of disk image * it is, and info about its capacity if available. */ -static int virStorageBackendProbeTarget(virConnectPtr conn, - virStorageVolTargetPtr target, - char **backingStore, - unsigned long long *allocation, - unsigned long long *capacity, - virStorageEncryptionPtr *encryption) { - int fd; +static int +virStorageGetMetadataFromFD(virConnectPtr conn, + const char *path, + int fd, + int *format, + bool *encrypted, + char **backingStore, + unsigned long long *capacity) +{ unsigned char head[20*512]; /* vmdk4GetBackingStore needs this much. */ - int len, i, ret; + int len, i;
+ if (format) /* If all else fails, call it a raw file */ + *format = VIR_STORAGE_FILE_RAW; + if (encrypted) + *encrypted = false; if (backingStore) *backingStore = NULL; - if (encryption) - *encryption = NULL; - - if ((fd = open(target->path, O_RDONLY)) < 0) { - virReportSystemError(conn, errno, - _("cannot open volume '%s'"), - target->path); - return -1; - } - - if ((ret = virStorageBackendUpdateVolTargetInfoFD(conn, target, fd, - allocation, - capacity)) < 0) { - close(fd); - return ret; /* Take care to propagate ret, it is not always -1 */ - } + /* Do not overwrite capacity + * if (capacity) + * *capacity = 0; + */
if ((len = read(fd, head, sizeof(head))) < 0) { - virReportSystemError(conn, errno, - _("cannot read header '%s'"), - target->path); - close(fd); + virReportSystemError(conn, errno, _("cannot read header '%s'"), path); return -1; }
- close(fd); - /* First check file magic */ for (i = 0 ; i < ARRAY_CARDINALITY(fileTypeInfo) ; i++) { int mlen; - bool encrypted_qcow = false;
if (fileTypeInfo[i].magic == NULL) continue; @@ -385,18 +373,19 @@ static int virStorageBackendProbeTarget(virConnectPtr conn, *capacity *= fileTypeInfo[i].sizeMultiplier; }
- if (fileTypeInfo[i].qcowCryptOffset != -1) { + if (fileTypeInfo[i].qcowCryptOffset != -1 && encrypted) { int crypt_format;
crypt_format = (head[fileTypeInfo[i].qcowCryptOffset] << 24) | (head[fileTypeInfo[i].qcowCryptOffset+1] << 16) | (head[fileTypeInfo[i].qcowCryptOffset+2] << 8) | head[fileTypeInfo[i].qcowCryptOffset+3]; - encrypted_qcow = crypt_format != 0; + *encrypted = crypt_format != 0; }
/* Validation passed, we know the file format now */ - target->format = fileTypeInfo[i].type; + if (format) + *format = fileTypeInfo[i].type; if (fileTypeInfo[i].getBackingStore != NULL && backingStore) { char *base;
@@ -411,8 +400,7 @@ static int virStorageBackendProbeTarget(virConnectPtr conn, return -1; } if (base != NULL) { - *backingStore - = absolutePathFromBaseFile(target->path, base); + *backingStore = absolutePathFromBaseFile(path, base); VIR_FREE(base); if (*backingStore == NULL) { virReportOOMError(conn); @@ -420,23 +408,6 @@ static int virStorageBackendProbeTarget(virConnectPtr conn, } } } - if (encryption != NULL && encrypted_qcow) { - virStorageEncryptionPtr enc; - - if (VIR_ALLOC(enc) < 0) { - virReportOOMError(conn); - if (backingStore) - VIR_FREE(*backingStore); - return -1; - } - enc->format = VIR_STORAGE_ENCRYPTION_FORMAT_QCOW; - *encryption = enc; - /* XXX ideally we'd fill in secret UUID here - * but we cannot guarentee 'conn' is non-NULL - * at this point in time :-( So we only fill - * in secrets when someone first queries a vol - */ - } return 0; }
@@ -445,15 +416,78 @@ static int virStorageBackendProbeTarget(virConnectPtr conn, if (fileTypeInfo[i].extension == NULL) continue;
- if (!virFileHasSuffix(target->path, fileTypeInfo[i].extension)) + if (!virFileHasSuffix(path, fileTypeInfo[i].extension)) continue;
- target->format = fileTypeInfo[i].type; + if (format) + *format = fileTypeInfo[i].type; return 0; }
- /* All fails, so call it a raw file */ - target->format = VIR_STORAGE_FILE_RAW; + return 0; +} + +static int +virStorageBackendProbeTarget(virConnectPtr conn, + virStorageVolTargetPtr target, + char **backingStore, + unsigned long long *allocation, + unsigned long long *capacity, + virStorageEncryptionPtr *encryption) +{ + int fd, ret; + bool encrypted; + + if (encryption) + *encryption = NULL; + + if ((fd = open(target->path, O_RDONLY)) < 0) { + virReportSystemError(conn, errno, + _("cannot open volume '%s'"), + target->path); + return -1; + } + + if ((ret = virStorageBackendUpdateVolTargetInfoFD(conn, target, fd, + allocation, + capacity)) < 0) { + close(fd); + return ret; /* Take care to propagate ret, it is not always -1 */ + } + + if (virStorageGetMetadataFromFD(conn, target->path, fd, + &target->format, &encrypted, + backingStore, capacity) < 0) { + close(fd); + return -1; + } + + close(fd); + + if (encryption != NULL && encrypted) { + if (VIR_ALLOC(*encryption) < 0) { + virReportOOMError(conn); + if (backingStore) + VIR_FREE(*backingStore); + return -1; + } + + switch (target->format) { + case VIR_STORAGE_FILE_QCOW: + case VIR_STORAGE_FILE_QCOW2: + (*encryption)->format = VIR_STORAGE_ENCRYPTION_FORMAT_QCOW; + break; + default: + break; + } + + /* XXX ideally we'd fill in secret UUID here + * but we cannot guarentee 'conn' is non-NULL + * at this point in time :-( So we only fill + * in secrets when someone first queries a vol + */ + } + return 0; }
ACK, this looks a lot nicer refactored Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Introduce a metadata structure and make virStorageGetMetadataFromFD() fill it in. * src/util/storage_file.h: add virStorageFileMetadata * src/backend/storage_backend_fs.c: virStorageGetMetadataFromFD() now fills in the virStorageFileMetadata structure --- src/storage/storage_backend_fs.c | 65 ++++++++++++++++++------------------- src/util/storage_file.h | 8 +++++ 2 files changed, 40 insertions(+), 33 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 87c30cd..7db73bf 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -287,24 +287,13 @@ static int virStorageGetMetadataFromFD(virConnectPtr conn, const char *path, int fd, - int *format, - bool *encrypted, - char **backingStore, - unsigned long long *capacity) + virStorageFileMetadata *meta) { unsigned char head[20*512]; /* vmdk4GetBackingStore needs this much. */ int len, i; - if (format) /* If all else fails, call it a raw file */ - *format = VIR_STORAGE_FILE_RAW; - if (encrypted) - *encrypted = false; - if (backingStore) - *backingStore = NULL; - /* Do not overwrite capacity - * if (capacity) - * *capacity = 0; - */ + /* If all else fails, call it a raw file */ + meta->format = VIR_STORAGE_FILE_RAW; if ((len = read(fd, head, sizeof(head))) < 0) { virReportSystemError(conn, errno, _("cannot read header '%s'"), path); @@ -345,9 +334,9 @@ virStorageGetMetadataFromFD(virConnectPtr conn, } /* Optionally extract capacity from file */ - if (fileTypeInfo[i].sizeOffset != -1 && capacity) { + if (fileTypeInfo[i].sizeOffset != -1) { if (fileTypeInfo[i].endian == LV_LITTLE_ENDIAN) { - *capacity = + meta->capacity = ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7] << 56) | ((unsigned long long)head[fileTypeInfo[i].sizeOffset+6] << 48) | ((unsigned long long)head[fileTypeInfo[i].sizeOffset+5] << 40) | @@ -357,7 +346,7 @@ virStorageGetMetadataFromFD(virConnectPtr conn, ((unsigned long long)head[fileTypeInfo[i].sizeOffset+1] << 8) | ((unsigned long long)head[fileTypeInfo[i].sizeOffset]); } else { - *capacity = + meta->capacity = ((unsigned long long)head[fileTypeInfo[i].sizeOffset] << 56) | ((unsigned long long)head[fileTypeInfo[i].sizeOffset+1] << 48) | ((unsigned long long)head[fileTypeInfo[i].sizeOffset+2] << 40) | @@ -368,25 +357,24 @@ virStorageGetMetadataFromFD(virConnectPtr conn, ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7]); } /* Avoid unlikely, but theoretically possible overflow */ - if (*capacity > (ULLONG_MAX / fileTypeInfo[i].sizeMultiplier)) + if (meta->capacity > (ULLONG_MAX / fileTypeInfo[i].sizeMultiplier)) continue; - *capacity *= fileTypeInfo[i].sizeMultiplier; + meta->capacity *= fileTypeInfo[i].sizeMultiplier; } - if (fileTypeInfo[i].qcowCryptOffset != -1 && encrypted) { + if (fileTypeInfo[i].qcowCryptOffset != -1) { int crypt_format; crypt_format = (head[fileTypeInfo[i].qcowCryptOffset] << 24) | (head[fileTypeInfo[i].qcowCryptOffset+1] << 16) | (head[fileTypeInfo[i].qcowCryptOffset+2] << 8) | head[fileTypeInfo[i].qcowCryptOffset+3]; - *encrypted = crypt_format != 0; + meta->encrypted = crypt_format != 0; } /* Validation passed, we know the file format now */ - if (format) - *format = fileTypeInfo[i].type; - if (fileTypeInfo[i].getBackingStore != NULL && backingStore) { + meta->format = fileTypeInfo[i].type; + if (fileTypeInfo[i].getBackingStore != NULL) { char *base; switch (fileTypeInfo[i].getBackingStore(conn, &base, head, len)) { @@ -400,9 +388,9 @@ virStorageGetMetadataFromFD(virConnectPtr conn, return -1; } if (base != NULL) { - *backingStore = absolutePathFromBaseFile(path, base); + meta->backingStore = absolutePathFromBaseFile(path, base); VIR_FREE(base); - if (*backingStore == NULL) { + if (meta->backingStore == NULL) { virReportOOMError(conn); return -1; } @@ -419,8 +407,7 @@ virStorageGetMetadataFromFD(virConnectPtr conn, if (!virFileHasSuffix(path, fileTypeInfo[i].extension)) continue; - if (format) - *format = fileTypeInfo[i].type; + meta->format = fileTypeInfo[i].type; return 0; } @@ -436,7 +423,7 @@ virStorageBackendProbeTarget(virConnectPtr conn, virStorageEncryptionPtr *encryption) { int fd, ret; - bool encrypted; + virStorageFileMetadata meta; if (encryption) *encryption = NULL; @@ -455,16 +442,28 @@ virStorageBackendProbeTarget(virConnectPtr conn, return ret; /* Take care to propagate ret, it is not always -1 */ } - if (virStorageGetMetadataFromFD(conn, target->path, fd, - &target->format, &encrypted, - backingStore, capacity) < 0) { + memset(&meta, 0, sizeof(meta)); + + if (virStorageGetMetadataFromFD(conn, target->path, fd, &meta) < 0) { close(fd); return -1; } close(fd); - if (encryption != NULL && encrypted) { + target->format = meta.format; + + if (backingStore) { + *backingStore = meta.backingStore; + meta.backingStore = NULL; + } + + VIR_FREE(meta.backingStore); + + if (capacity && meta.capacity) + *capacity = meta.capacity; + + if (encryption != NULL && meta.encrypted) { if (VIR_ALLOC(*encryption) < 0) { virReportOOMError(conn); if (backingStore) diff --git a/src/util/storage_file.h b/src/util/storage_file.h index 7bccbe4..b458c0e 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -25,6 +25,7 @@ #define __VIR_STORAGE_FILE_H__ #include "util.h" +#include <stdbool.h> enum virStorageFileFormat { VIR_STORAGE_FILE_RAW = 0, @@ -43,4 +44,11 @@ enum virStorageFileFormat { VIR_ENUM_DECL(virStorageFileFormat); +typedef struct _virStorageFileMetadata { + int format; + char *backingStore; + unsigned long long capacity; + bool encrypted; +} virStorageFileMetadata; + #endif /* __VIR_STORAGE_FILE_H__ */ -- 1.6.2.5

On Tue, Sep 29, 2009 at 09:56:46AM +0100, Mark McLoughlin wrote:
Introduce a metadata structure and make virStorageGetMetadataFromFD() fill it in.
* src/util/storage_file.h: add virStorageFileMetadata
* src/backend/storage_backend_fs.c: virStorageGetMetadataFromFD() now fills in the virStorageFileMetadata structure --- src/storage/storage_backend_fs.c | 65 ++++++++++++++++++------------------- src/util/storage_file.h | 8 +++++ 2 files changed, 40 insertions(+), 33 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 87c30cd..7db73bf 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -287,24 +287,13 @@ static int virStorageGetMetadataFromFD(virConnectPtr conn, const char *path, int fd, - int *format, - bool *encrypted, - char **backingStore, - unsigned long long *capacity) + virStorageFileMetadata *meta) { unsigned char head[20*512]; /* vmdk4GetBackingStore needs this much. */ int len, i;
- if (format) /* If all else fails, call it a raw file */ - *format = VIR_STORAGE_FILE_RAW; - if (encrypted) - *encrypted = false; - if (backingStore) - *backingStore = NULL; - /* Do not overwrite capacity - * if (capacity) - * *capacity = 0; - */ + /* If all else fails, call it a raw file */ + meta->format = VIR_STORAGE_FILE_RAW;
if ((len = read(fd, head, sizeof(head))) < 0) { virReportSystemError(conn, errno, _("cannot read header '%s'"), path); @@ -345,9 +334,9 @@ virStorageGetMetadataFromFD(virConnectPtr conn, }
/* Optionally extract capacity from file */ - if (fileTypeInfo[i].sizeOffset != -1 && capacity) { + if (fileTypeInfo[i].sizeOffset != -1) { if (fileTypeInfo[i].endian == LV_LITTLE_ENDIAN) { - *capacity = + meta->capacity = ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7] << 56) | ((unsigned long long)head[fileTypeInfo[i].sizeOffset+6] << 48) | ((unsigned long long)head[fileTypeInfo[i].sizeOffset+5] << 40) | @@ -357,7 +346,7 @@ virStorageGetMetadataFromFD(virConnectPtr conn, ((unsigned long long)head[fileTypeInfo[i].sizeOffset+1] << 8) | ((unsigned long long)head[fileTypeInfo[i].sizeOffset]); } else { - *capacity = + meta->capacity = ((unsigned long long)head[fileTypeInfo[i].sizeOffset] << 56) | ((unsigned long long)head[fileTypeInfo[i].sizeOffset+1] << 48) | ((unsigned long long)head[fileTypeInfo[i].sizeOffset+2] << 40) | @@ -368,25 +357,24 @@ virStorageGetMetadataFromFD(virConnectPtr conn, ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7]); } /* Avoid unlikely, but theoretically possible overflow */ - if (*capacity > (ULLONG_MAX / fileTypeInfo[i].sizeMultiplier)) + if (meta->capacity > (ULLONG_MAX / fileTypeInfo[i].sizeMultiplier)) continue; - *capacity *= fileTypeInfo[i].sizeMultiplier; + meta->capacity *= fileTypeInfo[i].sizeMultiplier; }
- if (fileTypeInfo[i].qcowCryptOffset != -1 && encrypted) { + if (fileTypeInfo[i].qcowCryptOffset != -1) { int crypt_format;
crypt_format = (head[fileTypeInfo[i].qcowCryptOffset] << 24) | (head[fileTypeInfo[i].qcowCryptOffset+1] << 16) | (head[fileTypeInfo[i].qcowCryptOffset+2] << 8) | head[fileTypeInfo[i].qcowCryptOffset+3]; - *encrypted = crypt_format != 0; + meta->encrypted = crypt_format != 0; }
/* Validation passed, we know the file format now */ - if (format) - *format = fileTypeInfo[i].type; - if (fileTypeInfo[i].getBackingStore != NULL && backingStore) { + meta->format = fileTypeInfo[i].type; + if (fileTypeInfo[i].getBackingStore != NULL) { char *base;
switch (fileTypeInfo[i].getBackingStore(conn, &base, head, len)) { @@ -400,9 +388,9 @@ virStorageGetMetadataFromFD(virConnectPtr conn, return -1; } if (base != NULL) { - *backingStore = absolutePathFromBaseFile(path, base); + meta->backingStore = absolutePathFromBaseFile(path, base); VIR_FREE(base); - if (*backingStore == NULL) { + if (meta->backingStore == NULL) { virReportOOMError(conn); return -1; } @@ -419,8 +407,7 @@ virStorageGetMetadataFromFD(virConnectPtr conn, if (!virFileHasSuffix(path, fileTypeInfo[i].extension)) continue;
- if (format) - *format = fileTypeInfo[i].type; + meta->format = fileTypeInfo[i].type; return 0; }
@@ -436,7 +423,7 @@ virStorageBackendProbeTarget(virConnectPtr conn, virStorageEncryptionPtr *encryption) { int fd, ret; - bool encrypted; + virStorageFileMetadata meta;
if (encryption) *encryption = NULL; @@ -455,16 +442,28 @@ virStorageBackendProbeTarget(virConnectPtr conn, return ret; /* Take care to propagate ret, it is not always -1 */ }
- if (virStorageGetMetadataFromFD(conn, target->path, fd, - &target->format, &encrypted, - backingStore, capacity) < 0) { + memset(&meta, 0, sizeof(meta)); + + if (virStorageGetMetadataFromFD(conn, target->path, fd, &meta) < 0) { close(fd); return -1; }
close(fd);
- if (encryption != NULL && encrypted) { + target->format = meta.format; + + if (backingStore) { + *backingStore = meta.backingStore; + meta.backingStore = NULL; + } + + VIR_FREE(meta.backingStore); + + if (capacity && meta.capacity) + *capacity = meta.capacity; + + if (encryption != NULL && meta.encrypted) { if (VIR_ALLOC(*encryption) < 0) { virReportOOMError(conn); if (backingStore) diff --git a/src/util/storage_file.h b/src/util/storage_file.h index 7bccbe4..b458c0e 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -25,6 +25,7 @@ #define __VIR_STORAGE_FILE_H__
#include "util.h" +#include <stdbool.h>
enum virStorageFileFormat { VIR_STORAGE_FILE_RAW = 0, @@ -43,4 +44,11 @@ enum virStorageFileFormat {
VIR_ENUM_DECL(virStorageFileFormat);
+typedef struct _virStorageFileMetadata { + int format; + char *backingStore; + unsigned long long capacity; + bool encrypted; +} virStorageFileMetadata; + #endif /* __VIR_STORAGE_FILE_H__ */
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Finally, we get to the point of all this. Move virStorageGetMetadataFromFD() to virStorageFileGetMetadataFromFD() and move to src/util/storage_file.[ch] There's no functional changes in this patch, just code movement * src/storage/storage_backend_fs.c: move code from here ... * src/util/storage_file.[ch]: ... to here * src/libvirt_private.syms: export virStorageFileGetMetadataFromFD() --- src/libvirt_private.syms | 1 + src/storage/storage_backend_fs.c | 368 +------------------------------------- src/util/storage_file.c | 373 ++++++++++++++++++++++++++++++++++++++ src/util/storage_file.h | 5 + 4 files changed, 380 insertions(+), 367 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ddab21d..4890032 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -393,6 +393,7 @@ virStorageGenerateQcowPassphrase; # storage_file.h virStorageFileFormatTypeToString; virStorageFileFormatTypeFromString; +virStorageFileGetMetadataFromFD; # threads.h virMutexInit; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 7db73bf..6816da8 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -46,375 +46,9 @@ #include "memory.h" #include "xml.h" -enum lv_endian { - LV_LITTLE_ENDIAN = 1, /* 1234 */ - LV_BIG_ENDIAN /* 4321 */ -}; - -enum { - BACKING_STORE_OK, - BACKING_STORE_INVALID, - BACKING_STORE_ERROR, -}; - -static int cowGetBackingStore(virConnectPtr, char **, - const unsigned char *, size_t); -static int qcowXGetBackingStore(virConnectPtr, char **, - const unsigned char *, size_t); -static int vmdk4GetBackingStore(virConnectPtr, char **, - const unsigned char *, size_t); - -/* Either 'magic' or 'extension' *must* be provided */ -struct FileTypeInfo { - int type; /* One of the constants above */ - const char *magic; /* Optional string of file magic - * to check at head of file */ - const char *extension; /* Optional file extension to check */ - enum lv_endian endian; /* Endianness of file format */ - int versionOffset; /* Byte offset from start of file - * where we find version number, - * -1 to skip version test */ - int versionNumber; /* Version number to validate */ - int sizeOffset; /* Byte offset from start of file - * where we find capacity info, - * -1 to use st_size as capacity */ - int sizeBytes; /* Number of bytes for size field */ - int sizeMultiplier; /* A scaling factor if size is not in bytes */ - /* Store a COW base image path (possibly relative), - * or NULL if there is no COW base image, to RES; - * return BACKING_STORE_* */ - int qcowCryptOffset; /* Byte offset from start of file - * where to find encryption mode, - * -1 if encryption is not used */ - int (*getBackingStore)(virConnectPtr conn, char **res, - const unsigned char *buf, size_t buf_size); -}; -struct FileTypeInfo const fileTypeInfo[] = { - /* Bochs */ - /* XXX Untested - { VIR_STORAGE_FILE_BOCHS, "Bochs Virtual HD Image", NULL, - LV_LITTLE_ENDIAN, 64, 0x20000, - 32+16+16+4+4+4+4+4, 8, 1, -1, NULL },*/ - /* CLoop */ - /* XXX Untested - { VIR_STORAGE_VOL_CLOOP, "#!/bin/sh\n#V2.0 Format\nmodprobe cloop file=$0 && mount -r -t iso9660 /dev/cloop $1\n", NULL, - LV_LITTLE_ENDIAN, -1, 0, - -1, 0, 0, -1, NULL }, */ - /* Cow */ - { VIR_STORAGE_FILE_COW, "OOOM", NULL, - LV_BIG_ENDIAN, 4, 2, - 4+4+1024+4, 8, 1, -1, cowGetBackingStore }, - /* DMG */ - /* XXX QEMU says there's no magic for dmg, but we should check... */ - { VIR_STORAGE_FILE_DMG, NULL, ".dmg", - 0, -1, 0, - -1, 0, 0, -1, NULL }, - /* XXX there's probably some magic for iso we can validate too... */ - { VIR_STORAGE_FILE_ISO, NULL, ".iso", - 0, -1, 0, - -1, 0, 0, -1, NULL }, - /* Parallels */ - /* XXX Untested - { VIR_STORAGE_FILE_PARALLELS, "WithoutFreeSpace", NULL, - LV_LITTLE_ENDIAN, 16, 2, - 16+4+4+4+4, 4, 512, -1, NULL }, - */ - /* QCow */ - { VIR_STORAGE_FILE_QCOW, "QFI", NULL, - LV_BIG_ENDIAN, 4, 1, - 4+4+8+4+4, 8, 1, 4+4+8+4+4+8+1+1+2, qcowXGetBackingStore }, - /* QCow 2 */ - { VIR_STORAGE_FILE_QCOW2, "QFI", NULL, - LV_BIG_ENDIAN, 4, 2, - 4+4+8+4+4, 8, 1, 4+4+8+4+4+8, qcowXGetBackingStore }, - /* VMDK 3 */ - /* XXX Untested - { VIR_STORAGE_FILE_VMDK, "COWD", NULL, - LV_LITTLE_ENDIAN, 4, 1, - 4+4+4, 4, 512, -1, NULL }, - */ - /* VMDK 4 */ - { VIR_STORAGE_FILE_VMDK, "KDMV", NULL, - LV_LITTLE_ENDIAN, 4, 1, - 4+4+4, 8, 512, -1, vmdk4GetBackingStore }, - /* Connectix / VirtualPC */ - /* XXX Untested - { VIR_STORAGE_FILE_VPC, "conectix", NULL, - LV_BIG_ENDIAN, -1, 0, - -1, 0, 0, -1, NULL}, - */ -}; - #define VIR_FROM_THIS VIR_FROM_STORAGE static int -cowGetBackingStore(virConnectPtr conn, - char **res, - const unsigned char *buf, - size_t buf_size) -{ -#define COW_FILENAME_MAXLEN 1024 - *res = NULL; - if (buf_size < 4+4+ COW_FILENAME_MAXLEN) - return BACKING_STORE_INVALID; - if (buf[4+4] == '\0') /* cow_header_v2.backing_file[0] */ - return BACKING_STORE_OK; - - *res = strndup ((const char*)buf + 4+4, COW_FILENAME_MAXLEN); - if (*res == NULL) { - virReportOOMError(conn); - return BACKING_STORE_ERROR; - } - return BACKING_STORE_OK; -} - -static int -qcowXGetBackingStore(virConnectPtr conn, - char **res, - const unsigned char *buf, - size_t buf_size) -{ - unsigned long long offset; - unsigned long size; - - *res = NULL; - if (buf_size < 4+4+8+4) - return BACKING_STORE_INVALID; - offset = (((unsigned long long)buf[4+4] << 56) - | ((unsigned long long)buf[4+4+1] << 48) - | ((unsigned long long)buf[4+4+2] << 40) - | ((unsigned long long)buf[4+4+3] << 32) - | ((unsigned long long)buf[4+4+4] << 24) - | ((unsigned long long)buf[4+4+5] << 16) - | ((unsigned long long)buf[4+4+6] << 8) - | buf[4+4+7]); /* QCowHeader.backing_file_offset */ - if (offset > buf_size) - return BACKING_STORE_INVALID; - size = ((buf[4+4+8] << 24) - | (buf[4+4+8+1] << 16) - | (buf[4+4+8+2] << 8) - | buf[4+4+8+3]); /* QCowHeader.backing_file_size */ - if (size == 0) - return BACKING_STORE_OK; - if (offset + size > buf_size || offset + size < offset) - return BACKING_STORE_INVALID; - if (size + 1 == 0) - return BACKING_STORE_INVALID; - if (VIR_ALLOC_N(*res, size + 1) < 0) { - virReportOOMError(conn); - return BACKING_STORE_ERROR; - } - memcpy(*res, buf + offset, size); - (*res)[size] = '\0'; - return BACKING_STORE_OK; -} - - -static int -vmdk4GetBackingStore(virConnectPtr conn, - char **res, - const unsigned char *buf, - size_t buf_size) -{ - static const char prefix[] = "parentFileNameHint=\""; - - char desc[20*512 + 1], *start, *end; - size_t len; - - *res = NULL; - - if (buf_size <= 0x200) - return BACKING_STORE_INVALID; - len = buf_size - 0x200; - if (len > sizeof(desc) - 1) - len = sizeof(desc) - 1; - memcpy(desc, buf + 0x200, len); - desc[len] = '\0'; - start = strstr(desc, prefix); - if (start == NULL) - return BACKING_STORE_OK; - start += strlen(prefix); - end = strchr(start, '"'); - if (end == NULL) - return BACKING_STORE_INVALID; - if (end == start) - return BACKING_STORE_OK; - *end = '\0'; - *res = strdup(start); - if (*res == NULL) { - virReportOOMError(conn); - return BACKING_STORE_ERROR; - } - return BACKING_STORE_OK; -} - -/** - * Return an absolute path corresponding to PATH, which is absolute or relative - * to the directory containing BASE_FILE, or NULL on error - */ -static char *absolutePathFromBaseFile(const char *base_file, const char *path) -{ - size_t base_size, path_size; - char *res, *p; - - if (*path == '/') - return strdup(path); - - base_size = strlen(base_file) + 1; - path_size = strlen(path) + 1; - if (VIR_ALLOC_N(res, base_size - 1 + path_size) < 0) - return NULL; - memcpy(res, base_file, base_size); - p = strrchr(res, '/'); - if (p != NULL) - p++; - else - p = res; - memcpy(p, path, path_size); - if (VIR_REALLOC_N(res, (p + path_size) - res) < 0) { - /* Ignore failure */ - } - return res; -} - - - -/** - * Probe the header of a file to determine what type of disk image - * it is, and info about its capacity if available. - */ -static int -virStorageGetMetadataFromFD(virConnectPtr conn, - const char *path, - int fd, - virStorageFileMetadata *meta) -{ - unsigned char head[20*512]; /* vmdk4GetBackingStore needs this much. */ - int len, i; - - /* If all else fails, call it a raw file */ - meta->format = VIR_STORAGE_FILE_RAW; - - if ((len = read(fd, head, sizeof(head))) < 0) { - virReportSystemError(conn, errno, _("cannot read header '%s'"), path); - return -1; - } - - /* First check file magic */ - for (i = 0 ; i < ARRAY_CARDINALITY(fileTypeInfo) ; i++) { - int mlen; - - if (fileTypeInfo[i].magic == NULL) - continue; - - /* Validate magic data */ - mlen = strlen(fileTypeInfo[i].magic); - if (mlen > len) - continue; - if (memcmp(head, fileTypeInfo[i].magic, mlen) != 0) - continue; - - /* Validate version number info */ - if (fileTypeInfo[i].versionNumber != -1) { - int version; - - if (fileTypeInfo[i].endian == LV_LITTLE_ENDIAN) { - version = (head[fileTypeInfo[i].versionOffset+3] << 24) | - (head[fileTypeInfo[i].versionOffset+2] << 16) | - (head[fileTypeInfo[i].versionOffset+1] << 8) | - head[fileTypeInfo[i].versionOffset]; - } else { - version = (head[fileTypeInfo[i].versionOffset] << 24) | - (head[fileTypeInfo[i].versionOffset+1] << 16) | - (head[fileTypeInfo[i].versionOffset+2] << 8) | - head[fileTypeInfo[i].versionOffset+3]; - } - if (version != fileTypeInfo[i].versionNumber) - continue; - } - - /* Optionally extract capacity from file */ - if (fileTypeInfo[i].sizeOffset != -1) { - if (fileTypeInfo[i].endian == LV_LITTLE_ENDIAN) { - meta->capacity = - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7] << 56) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+6] << 48) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+5] << 40) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+4] << 32) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+3] << 24) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+2] << 16) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+1] << 8) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset]); - } else { - meta->capacity = - ((unsigned long long)head[fileTypeInfo[i].sizeOffset] << 56) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+1] << 48) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+2] << 40) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+3] << 32) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+4] << 24) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+5] << 16) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+6] << 8) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7]); - } - /* Avoid unlikely, but theoretically possible overflow */ - if (meta->capacity > (ULLONG_MAX / fileTypeInfo[i].sizeMultiplier)) - continue; - meta->capacity *= fileTypeInfo[i].sizeMultiplier; - } - - if (fileTypeInfo[i].qcowCryptOffset != -1) { - int crypt_format; - - crypt_format = (head[fileTypeInfo[i].qcowCryptOffset] << 24) | - (head[fileTypeInfo[i].qcowCryptOffset+1] << 16) | - (head[fileTypeInfo[i].qcowCryptOffset+2] << 8) | - head[fileTypeInfo[i].qcowCryptOffset+3]; - meta->encrypted = crypt_format != 0; - } - - /* Validation passed, we know the file format now */ - meta->format = fileTypeInfo[i].type; - if (fileTypeInfo[i].getBackingStore != NULL) { - char *base; - - switch (fileTypeInfo[i].getBackingStore(conn, &base, head, len)) { - case BACKING_STORE_OK: - break; - - case BACKING_STORE_INVALID: - continue; - - case BACKING_STORE_ERROR: - return -1; - } - if (base != NULL) { - meta->backingStore = absolutePathFromBaseFile(path, base); - VIR_FREE(base); - if (meta->backingStore == NULL) { - virReportOOMError(conn); - return -1; - } - } - } - return 0; - } - - /* No magic, so check file extension */ - for (i = 0 ; i < ARRAY_CARDINALITY(fileTypeInfo) ; i++) { - if (fileTypeInfo[i].extension == NULL) - continue; - - if (!virFileHasSuffix(path, fileTypeInfo[i].extension)) - continue; - - meta->format = fileTypeInfo[i].type; - return 0; - } - - return 0; -} - -static int virStorageBackendProbeTarget(virConnectPtr conn, virStorageVolTargetPtr target, char **backingStore, @@ -444,7 +78,7 @@ virStorageBackendProbeTarget(virConnectPtr conn, memset(&meta, 0, sizeof(meta)); - if (virStorageGetMetadataFromFD(conn, target->path, fd, &meta) < 0) { + if (virStorageFileGetMetadataFromFD(conn, target->path, fd, &meta) < 0) { close(fd); return -1; } diff --git a/src/util/storage_file.c b/src/util/storage_file.c index e66ec8a..e674713 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -24,8 +24,381 @@ #include <config.h> #include "storage_file.h" +#include <unistd.h> +#include "memory.h" +#include "virterror_internal.h" + +#define VIR_FROM_THIS VIR_FROM_STORAGE + VIR_ENUM_IMPL(virStorageFileFormat, VIR_STORAGE_FILE_LAST, "raw", "dir", "bochs", "cloop", "cow", "dmg", "iso", "qcow", "qcow2", "vmdk", "vpc") + +enum lv_endian { + LV_LITTLE_ENDIAN = 1, /* 1234 */ + LV_BIG_ENDIAN /* 4321 */ +}; + +enum { + BACKING_STORE_OK, + BACKING_STORE_INVALID, + BACKING_STORE_ERROR, +}; + +/* Either 'magic' or 'extension' *must* be provided */ +struct FileTypeInfo { + int type; /* One of the constants above */ + const char *magic; /* Optional string of file magic + * to check at head of file */ + const char *extension; /* Optional file extension to check */ + enum lv_endian endian; /* Endianness of file format */ + int versionOffset; /* Byte offset from start of file + * where we find version number, + * -1 to skip version test */ + int versionNumber; /* Version number to validate */ + int sizeOffset; /* Byte offset from start of file + * where we find capacity info, + * -1 to use st_size as capacity */ + int sizeBytes; /* Number of bytes for size field */ + int sizeMultiplier; /* A scaling factor if size is not in bytes */ + /* Store a COW base image path (possibly relative), + * or NULL if there is no COW base image, to RES; + * return BACKING_STORE_* */ + int qcowCryptOffset; /* Byte offset from start of file + * where to find encryption mode, + * -1 if encryption is not used */ + int (*getBackingStore)(virConnectPtr conn, char **res, + const unsigned char *buf, size_t buf_size); +}; + +static int cowGetBackingStore(virConnectPtr, char **, + const unsigned char *, size_t); +static int qcowXGetBackingStore(virConnectPtr, char **, + const unsigned char *, size_t); +static int vmdk4GetBackingStore(virConnectPtr, char **, + const unsigned char *, size_t); + + +static struct FileTypeInfo const fileTypeInfo[] = { + /* Bochs */ + /* XXX Untested + { VIR_STORAGE_FILE_BOCHS, "Bochs Virtual HD Image", NULL, + LV_LITTLE_ENDIAN, 64, 0x20000, + 32+16+16+4+4+4+4+4, 8, 1, -1, NULL },*/ + /* CLoop */ + /* XXX Untested + { VIR_STORAGE_VOL_CLOOP, "#!/bin/sh\n#V2.0 Format\nmodprobe cloop file=$0 && mount -r -t iso9660 /dev/cloop $1\n", NULL, + LV_LITTLE_ENDIAN, -1, 0, + -1, 0, 0, -1, NULL }, */ + /* Cow */ + { VIR_STORAGE_FILE_COW, "OOOM", NULL, + LV_BIG_ENDIAN, 4, 2, + 4+4+1024+4, 8, 1, -1, cowGetBackingStore }, + /* DMG */ + /* XXX QEMU says there's no magic for dmg, but we should check... */ + { VIR_STORAGE_FILE_DMG, NULL, ".dmg", + 0, -1, 0, + -1, 0, 0, -1, NULL }, + /* XXX there's probably some magic for iso we can validate too... */ + { VIR_STORAGE_FILE_ISO, NULL, ".iso", + 0, -1, 0, + -1, 0, 0, -1, NULL }, + /* Parallels */ + /* XXX Untested + { VIR_STORAGE_FILE_PARALLELS, "WithoutFreeSpace", NULL, + LV_LITTLE_ENDIAN, 16, 2, + 16+4+4+4+4, 4, 512, -1, NULL }, + */ + /* QCow */ + { VIR_STORAGE_FILE_QCOW, "QFI", NULL, + LV_BIG_ENDIAN, 4, 1, + 4+4+8+4+4, 8, 1, 4+4+8+4+4+8+1+1+2, qcowXGetBackingStore }, + /* QCow 2 */ + { VIR_STORAGE_FILE_QCOW2, "QFI", NULL, + LV_BIG_ENDIAN, 4, 2, + 4+4+8+4+4, 8, 1, 4+4+8+4+4+8, qcowXGetBackingStore }, + /* VMDK 3 */ + /* XXX Untested + { VIR_STORAGE_FILE_VMDK, "COWD", NULL, + LV_LITTLE_ENDIAN, 4, 1, + 4+4+4, 4, 512, -1, NULL }, + */ + /* VMDK 4 */ + { VIR_STORAGE_FILE_VMDK, "KDMV", NULL, + LV_LITTLE_ENDIAN, 4, 1, + 4+4+4, 8, 512, -1, vmdk4GetBackingStore }, + /* Connectix / VirtualPC */ + /* XXX Untested + { VIR_STORAGE_FILE_VPC, "conectix", NULL, + LV_BIG_ENDIAN, -1, 0, + -1, 0, 0, -1, NULL}, + */ +}; + +static int +cowGetBackingStore(virConnectPtr conn, + char **res, + const unsigned char *buf, + size_t buf_size) +{ +#define COW_FILENAME_MAXLEN 1024 + *res = NULL; + if (buf_size < 4+4+ COW_FILENAME_MAXLEN) + return BACKING_STORE_INVALID; + if (buf[4+4] == '\0') /* cow_header_v2.backing_file[0] */ + return BACKING_STORE_OK; + + *res = strndup ((const char*)buf + 4+4, COW_FILENAME_MAXLEN); + if (*res == NULL) { + virReportOOMError(conn); + return BACKING_STORE_ERROR; + } + return BACKING_STORE_OK; +} + +static int +qcowXGetBackingStore(virConnectPtr conn, + char **res, + const unsigned char *buf, + size_t buf_size) +{ + unsigned long long offset; + unsigned long size; + + *res = NULL; + if (buf_size < 4+4+8+4) + return BACKING_STORE_INVALID; + offset = (((unsigned long long)buf[4+4] << 56) + | ((unsigned long long)buf[4+4+1] << 48) + | ((unsigned long long)buf[4+4+2] << 40) + | ((unsigned long long)buf[4+4+3] << 32) + | ((unsigned long long)buf[4+4+4] << 24) + | ((unsigned long long)buf[4+4+5] << 16) + | ((unsigned long long)buf[4+4+6] << 8) + | buf[4+4+7]); /* QCowHeader.backing_file_offset */ + if (offset > buf_size) + return BACKING_STORE_INVALID; + size = ((buf[4+4+8] << 24) + | (buf[4+4+8+1] << 16) + | (buf[4+4+8+2] << 8) + | buf[4+4+8+3]); /* QCowHeader.backing_file_size */ + if (size == 0) + return BACKING_STORE_OK; + if (offset + size > buf_size || offset + size < offset) + return BACKING_STORE_INVALID; + if (size + 1 == 0) + return BACKING_STORE_INVALID; + if (VIR_ALLOC_N(*res, size + 1) < 0) { + virReportOOMError(conn); + return BACKING_STORE_ERROR; + } + memcpy(*res, buf + offset, size); + (*res)[size] = '\0'; + return BACKING_STORE_OK; +} + + +static int +vmdk4GetBackingStore(virConnectPtr conn, + char **res, + const unsigned char *buf, + size_t buf_size) +{ + static const char prefix[] = "parentFileNameHint=\""; + + char desc[20*512 + 1], *start, *end; + size_t len; + + *res = NULL; + + if (buf_size <= 0x200) + return BACKING_STORE_INVALID; + len = buf_size - 0x200; + if (len > sizeof(desc) - 1) + len = sizeof(desc) - 1; + memcpy(desc, buf + 0x200, len); + desc[len] = '\0'; + start = strstr(desc, prefix); + if (start == NULL) + return BACKING_STORE_OK; + start += strlen(prefix); + end = strchr(start, '"'); + if (end == NULL) + return BACKING_STORE_INVALID; + if (end == start) + return BACKING_STORE_OK; + *end = '\0'; + *res = strdup(start); + if (*res == NULL) { + virReportOOMError(conn); + return BACKING_STORE_ERROR; + } + return BACKING_STORE_OK; +} + +/** + * Return an absolute path corresponding to PATH, which is absolute or relative + * to the directory containing BASE_FILE, or NULL on error + */ +static char * +absolutePathFromBaseFile(const char *base_file, const char *path) +{ + size_t base_size, path_size; + char *res, *p; + + if (*path == '/') + return strdup(path); + + base_size = strlen(base_file) + 1; + path_size = strlen(path) + 1; + if (VIR_ALLOC_N(res, base_size - 1 + path_size) < 0) + return NULL; + memcpy(res, base_file, base_size); + p = strrchr(res, '/'); + if (p != NULL) + p++; + else + p = res; + memcpy(p, path, path_size); + if (VIR_REALLOC_N(res, (p + path_size) - res) < 0) { + /* Ignore failure */ + } + return res; +} + +/** + * Probe the header of a file to determine what type of disk image + * it is, and info about its capacity if available. + */ +int +virStorageFileGetMetadataFromFD(virConnectPtr conn, + const char *path, + int fd, + virStorageFileMetadata *meta) +{ + unsigned char head[20*512]; /* vmdk4GetBackingStore needs this much. */ + int len, i; + + /* If all else fails, call it a raw file */ + meta->format = VIR_STORAGE_FILE_RAW; + + if ((len = read(fd, head, sizeof(head))) < 0) { + virReportSystemError(conn, errno, _("cannot read header '%s'"), path); + return -1; + } + + /* First check file magic */ + for (i = 0 ; i < ARRAY_CARDINALITY(fileTypeInfo) ; i++) { + int mlen; + + if (fileTypeInfo[i].magic == NULL) + continue; + + /* Validate magic data */ + mlen = strlen(fileTypeInfo[i].magic); + if (mlen > len) + continue; + if (memcmp(head, fileTypeInfo[i].magic, mlen) != 0) + continue; + + /* Validate version number info */ + if (fileTypeInfo[i].versionNumber != -1) { + int version; + + if (fileTypeInfo[i].endian == LV_LITTLE_ENDIAN) { + version = (head[fileTypeInfo[i].versionOffset+3] << 24) | + (head[fileTypeInfo[i].versionOffset+2] << 16) | + (head[fileTypeInfo[i].versionOffset+1] << 8) | + head[fileTypeInfo[i].versionOffset]; + } else { + version = (head[fileTypeInfo[i].versionOffset] << 24) | + (head[fileTypeInfo[i].versionOffset+1] << 16) | + (head[fileTypeInfo[i].versionOffset+2] << 8) | + head[fileTypeInfo[i].versionOffset+3]; + } + if (version != fileTypeInfo[i].versionNumber) + continue; + } + + /* Optionally extract capacity from file */ + if (fileTypeInfo[i].sizeOffset != -1) { + if (fileTypeInfo[i].endian == LV_LITTLE_ENDIAN) { + meta->capacity = + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7] << 56) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+6] << 48) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+5] << 40) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+4] << 32) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+3] << 24) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+2] << 16) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+1] << 8) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset]); + } else { + meta->capacity = + ((unsigned long long)head[fileTypeInfo[i].sizeOffset] << 56) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+1] << 48) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+2] << 40) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+3] << 32) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+4] << 24) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+5] << 16) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+6] << 8) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7]); + } + /* Avoid unlikely, but theoretically possible overflow */ + if (meta->capacity > (ULLONG_MAX / fileTypeInfo[i].sizeMultiplier)) + continue; + meta->capacity *= fileTypeInfo[i].sizeMultiplier; + } + + if (fileTypeInfo[i].qcowCryptOffset != -1) { + int crypt_format; + + crypt_format = (head[fileTypeInfo[i].qcowCryptOffset] << 24) | + (head[fileTypeInfo[i].qcowCryptOffset+1] << 16) | + (head[fileTypeInfo[i].qcowCryptOffset+2] << 8) | + head[fileTypeInfo[i].qcowCryptOffset+3]; + meta->encrypted = crypt_format != 0; + } + + /* Validation passed, we know the file format now */ + meta->format = fileTypeInfo[i].type; + if (fileTypeInfo[i].getBackingStore != NULL) { + char *base; + + switch (fileTypeInfo[i].getBackingStore(conn, &base, head, len)) { + case BACKING_STORE_OK: + break; + + case BACKING_STORE_INVALID: + continue; + + case BACKING_STORE_ERROR: + return -1; + } + if (base != NULL) { + meta->backingStore = absolutePathFromBaseFile(path, base); + VIR_FREE(base); + if (meta->backingStore == NULL) { + virReportOOMError(conn); + return -1; + } + } + } + return 0; + } + + /* No magic, so check file extension */ + for (i = 0 ; i < ARRAY_CARDINALITY(fileTypeInfo) ; i++) { + if (fileTypeInfo[i].extension == NULL) + continue; + + if (!virFileHasSuffix(path, fileTypeInfo[i].extension)) + continue; + + meta->format = fileTypeInfo[i].type; + return 0; + } + + return 0; +} diff --git a/src/util/storage_file.h b/src/util/storage_file.h index b458c0e..e34d749 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -51,4 +51,9 @@ typedef struct _virStorageFileMetadata { bool encrypted; } virStorageFileMetadata; +int virStorageFileGetMetadataFromFD(virConnectPtr conn, + const char *path, + int fd, + virStorageFileMetadata *meta); + #endif /* __VIR_STORAGE_FILE_H__ */ -- 1.6.2.5

On Tue, Sep 29, 2009 at 09:56:47AM +0100, Mark McLoughlin wrote:
Finally, we get to the point of all this.
Move virStorageGetMetadataFromFD() to virStorageFileGetMetadataFromFD() and move to src/util/storage_file.[ch]
There's no functional changes in this patch, just code movement
* src/storage/storage_backend_fs.c: move code from here ...
* src/util/storage_file.[ch]: ... to here
* src/libvirt_private.syms: export virStorageFileGetMetadataFromFD() --- src/libvirt_private.syms | 1 + src/storage/storage_backend_fs.c | 368 +------------------------------------- src/util/storage_file.c | 373 ++++++++++++++++++++++++++++++++++++++ src/util/storage_file.h | 5 + 4 files changed, 380 insertions(+), 367 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ddab21d..4890032 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -393,6 +393,7 @@ virStorageGenerateQcowPassphrase; # storage_file.h virStorageFileFormatTypeToString; virStorageFileFormatTypeFromString; +virStorageFileGetMetadataFromFD;
# threads.h virMutexInit; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 7db73bf..6816da8 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -46,375 +46,9 @@ #include "memory.h" #include "xml.h"
-enum lv_endian { - LV_LITTLE_ENDIAN = 1, /* 1234 */ - LV_BIG_ENDIAN /* 4321 */ -}; - -enum { - BACKING_STORE_OK, - BACKING_STORE_INVALID, - BACKING_STORE_ERROR, -}; - -static int cowGetBackingStore(virConnectPtr, char **, - const unsigned char *, size_t); -static int qcowXGetBackingStore(virConnectPtr, char **, - const unsigned char *, size_t); -static int vmdk4GetBackingStore(virConnectPtr, char **, - const unsigned char *, size_t); - -/* Either 'magic' or 'extension' *must* be provided */ -struct FileTypeInfo { - int type; /* One of the constants above */ - const char *magic; /* Optional string of file magic - * to check at head of file */ - const char *extension; /* Optional file extension to check */ - enum lv_endian endian; /* Endianness of file format */ - int versionOffset; /* Byte offset from start of file - * where we find version number, - * -1 to skip version test */ - int versionNumber; /* Version number to validate */ - int sizeOffset; /* Byte offset from start of file - * where we find capacity info, - * -1 to use st_size as capacity */ - int sizeBytes; /* Number of bytes for size field */ - int sizeMultiplier; /* A scaling factor if size is not in bytes */ - /* Store a COW base image path (possibly relative), - * or NULL if there is no COW base image, to RES; - * return BACKING_STORE_* */ - int qcowCryptOffset; /* Byte offset from start of file - * where to find encryption mode, - * -1 if encryption is not used */ - int (*getBackingStore)(virConnectPtr conn, char **res, - const unsigned char *buf, size_t buf_size); -}; -struct FileTypeInfo const fileTypeInfo[] = { - /* Bochs */ - /* XXX Untested - { VIR_STORAGE_FILE_BOCHS, "Bochs Virtual HD Image", NULL, - LV_LITTLE_ENDIAN, 64, 0x20000, - 32+16+16+4+4+4+4+4, 8, 1, -1, NULL },*/ - /* CLoop */ - /* XXX Untested - { VIR_STORAGE_VOL_CLOOP, "#!/bin/sh\n#V2.0 Format\nmodprobe cloop file=$0 && mount -r -t iso9660 /dev/cloop $1\n", NULL, - LV_LITTLE_ENDIAN, -1, 0, - -1, 0, 0, -1, NULL }, */ - /* Cow */ - { VIR_STORAGE_FILE_COW, "OOOM", NULL, - LV_BIG_ENDIAN, 4, 2, - 4+4+1024+4, 8, 1, -1, cowGetBackingStore }, - /* DMG */ - /* XXX QEMU says there's no magic for dmg, but we should check... */ - { VIR_STORAGE_FILE_DMG, NULL, ".dmg", - 0, -1, 0, - -1, 0, 0, -1, NULL }, - /* XXX there's probably some magic for iso we can validate too... */ - { VIR_STORAGE_FILE_ISO, NULL, ".iso", - 0, -1, 0, - -1, 0, 0, -1, NULL }, - /* Parallels */ - /* XXX Untested - { VIR_STORAGE_FILE_PARALLELS, "WithoutFreeSpace", NULL, - LV_LITTLE_ENDIAN, 16, 2, - 16+4+4+4+4, 4, 512, -1, NULL }, - */ - /* QCow */ - { VIR_STORAGE_FILE_QCOW, "QFI", NULL, - LV_BIG_ENDIAN, 4, 1, - 4+4+8+4+4, 8, 1, 4+4+8+4+4+8+1+1+2, qcowXGetBackingStore }, - /* QCow 2 */ - { VIR_STORAGE_FILE_QCOW2, "QFI", NULL, - LV_BIG_ENDIAN, 4, 2, - 4+4+8+4+4, 8, 1, 4+4+8+4+4+8, qcowXGetBackingStore }, - /* VMDK 3 */ - /* XXX Untested - { VIR_STORAGE_FILE_VMDK, "COWD", NULL, - LV_LITTLE_ENDIAN, 4, 1, - 4+4+4, 4, 512, -1, NULL }, - */ - /* VMDK 4 */ - { VIR_STORAGE_FILE_VMDK, "KDMV", NULL, - LV_LITTLE_ENDIAN, 4, 1, - 4+4+4, 8, 512, -1, vmdk4GetBackingStore }, - /* Connectix / VirtualPC */ - /* XXX Untested - { VIR_STORAGE_FILE_VPC, "conectix", NULL, - LV_BIG_ENDIAN, -1, 0, - -1, 0, 0, -1, NULL}, - */ -}; - #define VIR_FROM_THIS VIR_FROM_STORAGE
static int -cowGetBackingStore(virConnectPtr conn, - char **res, - const unsigned char *buf, - size_t buf_size) -{ -#define COW_FILENAME_MAXLEN 1024 - *res = NULL; - if (buf_size < 4+4+ COW_FILENAME_MAXLEN) - return BACKING_STORE_INVALID; - if (buf[4+4] == '\0') /* cow_header_v2.backing_file[0] */ - return BACKING_STORE_OK; - - *res = strndup ((const char*)buf + 4+4, COW_FILENAME_MAXLEN); - if (*res == NULL) { - virReportOOMError(conn); - return BACKING_STORE_ERROR; - } - return BACKING_STORE_OK; -} - -static int -qcowXGetBackingStore(virConnectPtr conn, - char **res, - const unsigned char *buf, - size_t buf_size) -{ - unsigned long long offset; - unsigned long size; - - *res = NULL; - if (buf_size < 4+4+8+4) - return BACKING_STORE_INVALID; - offset = (((unsigned long long)buf[4+4] << 56) - | ((unsigned long long)buf[4+4+1] << 48) - | ((unsigned long long)buf[4+4+2] << 40) - | ((unsigned long long)buf[4+4+3] << 32) - | ((unsigned long long)buf[4+4+4] << 24) - | ((unsigned long long)buf[4+4+5] << 16) - | ((unsigned long long)buf[4+4+6] << 8) - | buf[4+4+7]); /* QCowHeader.backing_file_offset */ - if (offset > buf_size) - return BACKING_STORE_INVALID; - size = ((buf[4+4+8] << 24) - | (buf[4+4+8+1] << 16) - | (buf[4+4+8+2] << 8) - | buf[4+4+8+3]); /* QCowHeader.backing_file_size */ - if (size == 0) - return BACKING_STORE_OK; - if (offset + size > buf_size || offset + size < offset) - return BACKING_STORE_INVALID; - if (size + 1 == 0) - return BACKING_STORE_INVALID; - if (VIR_ALLOC_N(*res, size + 1) < 0) { - virReportOOMError(conn); - return BACKING_STORE_ERROR; - } - memcpy(*res, buf + offset, size); - (*res)[size] = '\0'; - return BACKING_STORE_OK; -} - - -static int -vmdk4GetBackingStore(virConnectPtr conn, - char **res, - const unsigned char *buf, - size_t buf_size) -{ - static const char prefix[] = "parentFileNameHint=\""; - - char desc[20*512 + 1], *start, *end; - size_t len; - - *res = NULL; - - if (buf_size <= 0x200) - return BACKING_STORE_INVALID; - len = buf_size - 0x200; - if (len > sizeof(desc) - 1) - len = sizeof(desc) - 1; - memcpy(desc, buf + 0x200, len); - desc[len] = '\0'; - start = strstr(desc, prefix); - if (start == NULL) - return BACKING_STORE_OK; - start += strlen(prefix); - end = strchr(start, '"'); - if (end == NULL) - return BACKING_STORE_INVALID; - if (end == start) - return BACKING_STORE_OK; - *end = '\0'; - *res = strdup(start); - if (*res == NULL) { - virReportOOMError(conn); - return BACKING_STORE_ERROR; - } - return BACKING_STORE_OK; -} - -/** - * Return an absolute path corresponding to PATH, which is absolute or relative - * to the directory containing BASE_FILE, or NULL on error - */ -static char *absolutePathFromBaseFile(const char *base_file, const char *path) -{ - size_t base_size, path_size; - char *res, *p; - - if (*path == '/') - return strdup(path); - - base_size = strlen(base_file) + 1; - path_size = strlen(path) + 1; - if (VIR_ALLOC_N(res, base_size - 1 + path_size) < 0) - return NULL; - memcpy(res, base_file, base_size); - p = strrchr(res, '/'); - if (p != NULL) - p++; - else - p = res; - memcpy(p, path, path_size); - if (VIR_REALLOC_N(res, (p + path_size) - res) < 0) { - /* Ignore failure */ - } - return res; -} - - - -/** - * Probe the header of a file to determine what type of disk image - * it is, and info about its capacity if available. - */ -static int -virStorageGetMetadataFromFD(virConnectPtr conn, - const char *path, - int fd, - virStorageFileMetadata *meta) -{ - unsigned char head[20*512]; /* vmdk4GetBackingStore needs this much. */ - int len, i; - - /* If all else fails, call it a raw file */ - meta->format = VIR_STORAGE_FILE_RAW; - - if ((len = read(fd, head, sizeof(head))) < 0) { - virReportSystemError(conn, errno, _("cannot read header '%s'"), path); - return -1; - } - - /* First check file magic */ - for (i = 0 ; i < ARRAY_CARDINALITY(fileTypeInfo) ; i++) { - int mlen; - - if (fileTypeInfo[i].magic == NULL) - continue; - - /* Validate magic data */ - mlen = strlen(fileTypeInfo[i].magic); - if (mlen > len) - continue; - if (memcmp(head, fileTypeInfo[i].magic, mlen) != 0) - continue; - - /* Validate version number info */ - if (fileTypeInfo[i].versionNumber != -1) { - int version; - - if (fileTypeInfo[i].endian == LV_LITTLE_ENDIAN) { - version = (head[fileTypeInfo[i].versionOffset+3] << 24) | - (head[fileTypeInfo[i].versionOffset+2] << 16) | - (head[fileTypeInfo[i].versionOffset+1] << 8) | - head[fileTypeInfo[i].versionOffset]; - } else { - version = (head[fileTypeInfo[i].versionOffset] << 24) | - (head[fileTypeInfo[i].versionOffset+1] << 16) | - (head[fileTypeInfo[i].versionOffset+2] << 8) | - head[fileTypeInfo[i].versionOffset+3]; - } - if (version != fileTypeInfo[i].versionNumber) - continue; - } - - /* Optionally extract capacity from file */ - if (fileTypeInfo[i].sizeOffset != -1) { - if (fileTypeInfo[i].endian == LV_LITTLE_ENDIAN) { - meta->capacity = - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7] << 56) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+6] << 48) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+5] << 40) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+4] << 32) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+3] << 24) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+2] << 16) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+1] << 8) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset]); - } else { - meta->capacity = - ((unsigned long long)head[fileTypeInfo[i].sizeOffset] << 56) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+1] << 48) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+2] << 40) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+3] << 32) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+4] << 24) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+5] << 16) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+6] << 8) | - ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7]); - } - /* Avoid unlikely, but theoretically possible overflow */ - if (meta->capacity > (ULLONG_MAX / fileTypeInfo[i].sizeMultiplier)) - continue; - meta->capacity *= fileTypeInfo[i].sizeMultiplier; - } - - if (fileTypeInfo[i].qcowCryptOffset != -1) { - int crypt_format; - - crypt_format = (head[fileTypeInfo[i].qcowCryptOffset] << 24) | - (head[fileTypeInfo[i].qcowCryptOffset+1] << 16) | - (head[fileTypeInfo[i].qcowCryptOffset+2] << 8) | - head[fileTypeInfo[i].qcowCryptOffset+3]; - meta->encrypted = crypt_format != 0; - } - - /* Validation passed, we know the file format now */ - meta->format = fileTypeInfo[i].type; - if (fileTypeInfo[i].getBackingStore != NULL) { - char *base; - - switch (fileTypeInfo[i].getBackingStore(conn, &base, head, len)) { - case BACKING_STORE_OK: - break; - - case BACKING_STORE_INVALID: - continue; - - case BACKING_STORE_ERROR: - return -1; - } - if (base != NULL) { - meta->backingStore = absolutePathFromBaseFile(path, base); - VIR_FREE(base); - if (meta->backingStore == NULL) { - virReportOOMError(conn); - return -1; - } - } - } - return 0; - } - - /* No magic, so check file extension */ - for (i = 0 ; i < ARRAY_CARDINALITY(fileTypeInfo) ; i++) { - if (fileTypeInfo[i].extension == NULL) - continue; - - if (!virFileHasSuffix(path, fileTypeInfo[i].extension)) - continue; - - meta->format = fileTypeInfo[i].type; - return 0; - } - - return 0; -} - -static int virStorageBackendProbeTarget(virConnectPtr conn, virStorageVolTargetPtr target, char **backingStore, @@ -444,7 +78,7 @@ virStorageBackendProbeTarget(virConnectPtr conn,
memset(&meta, 0, sizeof(meta));
- if (virStorageGetMetadataFromFD(conn, target->path, fd, &meta) < 0) { + if (virStorageFileGetMetadataFromFD(conn, target->path, fd, &meta) < 0) { close(fd); return -1; } diff --git a/src/util/storage_file.c b/src/util/storage_file.c index e66ec8a..e674713 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -24,8 +24,381 @@ #include <config.h> #include "storage_file.h"
+#include <unistd.h> +#include "memory.h" +#include "virterror_internal.h" + +#define VIR_FROM_THIS VIR_FROM_STORAGE + VIR_ENUM_IMPL(virStorageFileFormat, VIR_STORAGE_FILE_LAST, "raw", "dir", "bochs", "cloop", "cow", "dmg", "iso", "qcow", "qcow2", "vmdk", "vpc") + +enum lv_endian { + LV_LITTLE_ENDIAN = 1, /* 1234 */ + LV_BIG_ENDIAN /* 4321 */ +}; + +enum { + BACKING_STORE_OK, + BACKING_STORE_INVALID, + BACKING_STORE_ERROR, +}; + +/* Either 'magic' or 'extension' *must* be provided */ +struct FileTypeInfo { + int type; /* One of the constants above */ + const char *magic; /* Optional string of file magic + * to check at head of file */ + const char *extension; /* Optional file extension to check */ + enum lv_endian endian; /* Endianness of file format */ + int versionOffset; /* Byte offset from start of file + * where we find version number, + * -1 to skip version test */ + int versionNumber; /* Version number to validate */ + int sizeOffset; /* Byte offset from start of file + * where we find capacity info, + * -1 to use st_size as capacity */ + int sizeBytes; /* Number of bytes for size field */ + int sizeMultiplier; /* A scaling factor if size is not in bytes */ + /* Store a COW base image path (possibly relative), + * or NULL if there is no COW base image, to RES; + * return BACKING_STORE_* */ + int qcowCryptOffset; /* Byte offset from start of file + * where to find encryption mode, + * -1 if encryption is not used */ + int (*getBackingStore)(virConnectPtr conn, char **res, + const unsigned char *buf, size_t buf_size); +}; + +static int cowGetBackingStore(virConnectPtr, char **, + const unsigned char *, size_t); +static int qcowXGetBackingStore(virConnectPtr, char **, + const unsigned char *, size_t); +static int vmdk4GetBackingStore(virConnectPtr, char **, + const unsigned char *, size_t); + + +static struct FileTypeInfo const fileTypeInfo[] = { + /* Bochs */ + /* XXX Untested + { VIR_STORAGE_FILE_BOCHS, "Bochs Virtual HD Image", NULL, + LV_LITTLE_ENDIAN, 64, 0x20000, + 32+16+16+4+4+4+4+4, 8, 1, -1, NULL },*/ + /* CLoop */ + /* XXX Untested + { VIR_STORAGE_VOL_CLOOP, "#!/bin/sh\n#V2.0 Format\nmodprobe cloop file=$0 && mount -r -t iso9660 /dev/cloop $1\n", NULL, + LV_LITTLE_ENDIAN, -1, 0, + -1, 0, 0, -1, NULL }, */ + /* Cow */ + { VIR_STORAGE_FILE_COW, "OOOM", NULL, + LV_BIG_ENDIAN, 4, 2, + 4+4+1024+4, 8, 1, -1, cowGetBackingStore }, + /* DMG */ + /* XXX QEMU says there's no magic for dmg, but we should check... */ + { VIR_STORAGE_FILE_DMG, NULL, ".dmg", + 0, -1, 0, + -1, 0, 0, -1, NULL }, + /* XXX there's probably some magic for iso we can validate too... */ + { VIR_STORAGE_FILE_ISO, NULL, ".iso", + 0, -1, 0, + -1, 0, 0, -1, NULL }, + /* Parallels */ + /* XXX Untested + { VIR_STORAGE_FILE_PARALLELS, "WithoutFreeSpace", NULL, + LV_LITTLE_ENDIAN, 16, 2, + 16+4+4+4+4, 4, 512, -1, NULL }, + */ + /* QCow */ + { VIR_STORAGE_FILE_QCOW, "QFI", NULL, + LV_BIG_ENDIAN, 4, 1, + 4+4+8+4+4, 8, 1, 4+4+8+4+4+8+1+1+2, qcowXGetBackingStore }, + /* QCow 2 */ + { VIR_STORAGE_FILE_QCOW2, "QFI", NULL, + LV_BIG_ENDIAN, 4, 2, + 4+4+8+4+4, 8, 1, 4+4+8+4+4+8, qcowXGetBackingStore }, + /* VMDK 3 */ + /* XXX Untested + { VIR_STORAGE_FILE_VMDK, "COWD", NULL, + LV_LITTLE_ENDIAN, 4, 1, + 4+4+4, 4, 512, -1, NULL }, + */ + /* VMDK 4 */ + { VIR_STORAGE_FILE_VMDK, "KDMV", NULL, + LV_LITTLE_ENDIAN, 4, 1, + 4+4+4, 8, 512, -1, vmdk4GetBackingStore }, + /* Connectix / VirtualPC */ + /* XXX Untested + { VIR_STORAGE_FILE_VPC, "conectix", NULL, + LV_BIG_ENDIAN, -1, 0, + -1, 0, 0, -1, NULL}, + */ +}; + +static int +cowGetBackingStore(virConnectPtr conn, + char **res, + const unsigned char *buf, + size_t buf_size) +{ +#define COW_FILENAME_MAXLEN 1024 + *res = NULL; + if (buf_size < 4+4+ COW_FILENAME_MAXLEN) + return BACKING_STORE_INVALID; + if (buf[4+4] == '\0') /* cow_header_v2.backing_file[0] */ + return BACKING_STORE_OK; + + *res = strndup ((const char*)buf + 4+4, COW_FILENAME_MAXLEN); + if (*res == NULL) { + virReportOOMError(conn); + return BACKING_STORE_ERROR; + } + return BACKING_STORE_OK; +} + +static int +qcowXGetBackingStore(virConnectPtr conn, + char **res, + const unsigned char *buf, + size_t buf_size) +{ + unsigned long long offset; + unsigned long size; + + *res = NULL; + if (buf_size < 4+4+8+4) + return BACKING_STORE_INVALID; + offset = (((unsigned long long)buf[4+4] << 56) + | ((unsigned long long)buf[4+4+1] << 48) + | ((unsigned long long)buf[4+4+2] << 40) + | ((unsigned long long)buf[4+4+3] << 32) + | ((unsigned long long)buf[4+4+4] << 24) + | ((unsigned long long)buf[4+4+5] << 16) + | ((unsigned long long)buf[4+4+6] << 8) + | buf[4+4+7]); /* QCowHeader.backing_file_offset */ + if (offset > buf_size) + return BACKING_STORE_INVALID; + size = ((buf[4+4+8] << 24) + | (buf[4+4+8+1] << 16) + | (buf[4+4+8+2] << 8) + | buf[4+4+8+3]); /* QCowHeader.backing_file_size */ + if (size == 0) + return BACKING_STORE_OK; + if (offset + size > buf_size || offset + size < offset) + return BACKING_STORE_INVALID; + if (size + 1 == 0) + return BACKING_STORE_INVALID; + if (VIR_ALLOC_N(*res, size + 1) < 0) { + virReportOOMError(conn); + return BACKING_STORE_ERROR; + } + memcpy(*res, buf + offset, size); + (*res)[size] = '\0'; + return BACKING_STORE_OK; +} + + +static int +vmdk4GetBackingStore(virConnectPtr conn, + char **res, + const unsigned char *buf, + size_t buf_size) +{ + static const char prefix[] = "parentFileNameHint=\""; + + char desc[20*512 + 1], *start, *end; + size_t len; + + *res = NULL; + + if (buf_size <= 0x200) + return BACKING_STORE_INVALID; + len = buf_size - 0x200; + if (len > sizeof(desc) - 1) + len = sizeof(desc) - 1; + memcpy(desc, buf + 0x200, len); + desc[len] = '\0'; + start = strstr(desc, prefix); + if (start == NULL) + return BACKING_STORE_OK; + start += strlen(prefix); + end = strchr(start, '"'); + if (end == NULL) + return BACKING_STORE_INVALID; + if (end == start) + return BACKING_STORE_OK; + *end = '\0'; + *res = strdup(start); + if (*res == NULL) { + virReportOOMError(conn); + return BACKING_STORE_ERROR; + } + return BACKING_STORE_OK; +} + +/** + * Return an absolute path corresponding to PATH, which is absolute or relative + * to the directory containing BASE_FILE, or NULL on error + */ +static char * +absolutePathFromBaseFile(const char *base_file, const char *path) +{ + size_t base_size, path_size; + char *res, *p; + + if (*path == '/') + return strdup(path); + + base_size = strlen(base_file) + 1; + path_size = strlen(path) + 1; + if (VIR_ALLOC_N(res, base_size - 1 + path_size) < 0) + return NULL; + memcpy(res, base_file, base_size); + p = strrchr(res, '/'); + if (p != NULL) + p++; + else + p = res; + memcpy(p, path, path_size); + if (VIR_REALLOC_N(res, (p + path_size) - res) < 0) { + /* Ignore failure */ + } + return res; +} + +/** + * Probe the header of a file to determine what type of disk image + * it is, and info about its capacity if available. + */ +int +virStorageFileGetMetadataFromFD(virConnectPtr conn, + const char *path, + int fd, + virStorageFileMetadata *meta) +{ + unsigned char head[20*512]; /* vmdk4GetBackingStore needs this much. */ + int len, i; + + /* If all else fails, call it a raw file */ + meta->format = VIR_STORAGE_FILE_RAW; + + if ((len = read(fd, head, sizeof(head))) < 0) { + virReportSystemError(conn, errno, _("cannot read header '%s'"), path); + return -1; + } + + /* First check file magic */ + for (i = 0 ; i < ARRAY_CARDINALITY(fileTypeInfo) ; i++) { + int mlen; + + if (fileTypeInfo[i].magic == NULL) + continue; + + /* Validate magic data */ + mlen = strlen(fileTypeInfo[i].magic); + if (mlen > len) + continue; + if (memcmp(head, fileTypeInfo[i].magic, mlen) != 0) + continue; + + /* Validate version number info */ + if (fileTypeInfo[i].versionNumber != -1) { + int version; + + if (fileTypeInfo[i].endian == LV_LITTLE_ENDIAN) { + version = (head[fileTypeInfo[i].versionOffset+3] << 24) | + (head[fileTypeInfo[i].versionOffset+2] << 16) | + (head[fileTypeInfo[i].versionOffset+1] << 8) | + head[fileTypeInfo[i].versionOffset]; + } else { + version = (head[fileTypeInfo[i].versionOffset] << 24) | + (head[fileTypeInfo[i].versionOffset+1] << 16) | + (head[fileTypeInfo[i].versionOffset+2] << 8) | + head[fileTypeInfo[i].versionOffset+3]; + } + if (version != fileTypeInfo[i].versionNumber) + continue; + } + + /* Optionally extract capacity from file */ + if (fileTypeInfo[i].sizeOffset != -1) { + if (fileTypeInfo[i].endian == LV_LITTLE_ENDIAN) { + meta->capacity = + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7] << 56) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+6] << 48) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+5] << 40) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+4] << 32) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+3] << 24) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+2] << 16) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+1] << 8) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset]); + } else { + meta->capacity = + ((unsigned long long)head[fileTypeInfo[i].sizeOffset] << 56) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+1] << 48) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+2] << 40) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+3] << 32) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+4] << 24) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+5] << 16) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+6] << 8) | + ((unsigned long long)head[fileTypeInfo[i].sizeOffset+7]); + } + /* Avoid unlikely, but theoretically possible overflow */ + if (meta->capacity > (ULLONG_MAX / fileTypeInfo[i].sizeMultiplier)) + continue; + meta->capacity *= fileTypeInfo[i].sizeMultiplier; + } + + if (fileTypeInfo[i].qcowCryptOffset != -1) { + int crypt_format; + + crypt_format = (head[fileTypeInfo[i].qcowCryptOffset] << 24) | + (head[fileTypeInfo[i].qcowCryptOffset+1] << 16) | + (head[fileTypeInfo[i].qcowCryptOffset+2] << 8) | + head[fileTypeInfo[i].qcowCryptOffset+3]; + meta->encrypted = crypt_format != 0; + } + + /* Validation passed, we know the file format now */ + meta->format = fileTypeInfo[i].type; + if (fileTypeInfo[i].getBackingStore != NULL) { + char *base; + + switch (fileTypeInfo[i].getBackingStore(conn, &base, head, len)) { + case BACKING_STORE_OK: + break; + + case BACKING_STORE_INVALID: + continue; + + case BACKING_STORE_ERROR: + return -1; + } + if (base != NULL) { + meta->backingStore = absolutePathFromBaseFile(path, base); + VIR_FREE(base); + if (meta->backingStore == NULL) { + virReportOOMError(conn); + return -1; + } + } + } + return 0; + } + + /* No magic, so check file extension */ + for (i = 0 ; i < ARRAY_CARDINALITY(fileTypeInfo) ; i++) { + if (fileTypeInfo[i].extension == NULL) + continue; + + if (!virFileHasSuffix(path, fileTypeInfo[i].extension)) + continue; + + meta->format = fileTypeInfo[i].type; + return 0; + } + + return 0; +} diff --git a/src/util/storage_file.h b/src/util/storage_file.h index b458c0e..e34d749 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -51,4 +51,9 @@ typedef struct _virStorageFileMetadata { bool encrypted; } virStorageFileMetadata;
+int virStorageFileGetMetadataFromFD(virConnectPtr conn, + const char *path, + int fd, + virStorageFileMetadata *meta); + #endif /* __VIR_STORAGE_FILE_H__ */
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

* src/util/storage_file.c: add virStorageFileGetMetadata() so that the caller does not need to open the file --- src/libvirt_private.syms | 1 + src/util/storage_file.c | 20 ++++++++++++++++++++ src/util/storage_file.h | 3 +++ 3 files changed, 24 insertions(+), 0 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4890032..635c6b6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -393,6 +393,7 @@ virStorageGenerateQcowPassphrase; # storage_file.h virStorageFileFormatTypeToString; virStorageFileFormatTypeFromString; +virStorageFileGetMetadata; virStorageFileGetMetadataFromFD; # threads.h diff --git a/src/util/storage_file.c b/src/util/storage_file.c index e674713..44057d2 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -25,6 +25,7 @@ #include "storage_file.h" #include <unistd.h> +#include <fcntl.h> #include "memory.h" #include "virterror_internal.h" @@ -402,3 +403,22 @@ virStorageFileGetMetadataFromFD(virConnectPtr conn, return 0; } + +int +virStorageFileGetMetadata(virConnectPtr conn, + const char *path, + virStorageFileMetadata *meta) +{ + int fd, ret; + + if ((fd = open(path, O_RDONLY)) < 0) { + virReportSystemError(conn, errno, _("cannot open file '%s'"), path); + return -1; + } + + ret = virStorageFileGetMetadataFromFD(conn, path, fd, meta); + + close(fd); + + return ret; +} diff --git a/src/util/storage_file.h b/src/util/storage_file.h index e34d749..b0abcaf 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -51,6 +51,9 @@ typedef struct _virStorageFileMetadata { bool encrypted; } virStorageFileMetadata; +int virStorageFileGetMetadata(virConnectPtr conn, + const char *path, + virStorageFileMetadata *meta); int virStorageFileGetMetadataFromFD(virConnectPtr conn, const char *path, int fd, -- 1.6.2.5

On Tue, Sep 29, 2009 at 09:56:48AM +0100, Mark McLoughlin wrote:
* src/util/storage_file.c: add virStorageFileGetMetadata() so that the caller does not need to open the file --- src/libvirt_private.syms | 1 + src/util/storage_file.c | 20 ++++++++++++++++++++ src/util/storage_file.h | 3 +++ 3 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4890032..635c6b6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -393,6 +393,7 @@ virStorageGenerateQcowPassphrase; # storage_file.h virStorageFileFormatTypeToString; virStorageFileFormatTypeFromString; +virStorageFileGetMetadata; virStorageFileGetMetadataFromFD;
# threads.h diff --git a/src/util/storage_file.c b/src/util/storage_file.c index e674713..44057d2 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -25,6 +25,7 @@ #include "storage_file.h"
#include <unistd.h> +#include <fcntl.h> #include "memory.h" #include "virterror_internal.h"
@@ -402,3 +403,22 @@ virStorageFileGetMetadataFromFD(virConnectPtr conn,
return 0; } + +int +virStorageFileGetMetadata(virConnectPtr conn, + const char *path, + virStorageFileMetadata *meta) +{ + int fd, ret; + + if ((fd = open(path, O_RDONLY)) < 0) { + virReportSystemError(conn, errno, _("cannot open file '%s'"), path); + return -1; + } + + ret = virStorageFileGetMetadataFromFD(conn, path, fd, meta); + + close(fd); + + return ret; +} diff --git a/src/util/storage_file.h b/src/util/storage_file.h index e34d749..b0abcaf 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -51,6 +51,9 @@ typedef struct _virStorageFileMetadata { bool encrypted; } virStorageFileMetadata;
+int virStorageFileGetMetadata(virConnectPtr conn, + const char *path, + virStorageFileMetadata *meta); int virStorageFileGetMetadataFromFD(virConnectPtr conn, const char *path, int fd,
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Use virStorageFileGetMetadata() to find any backing stores for images and re-label them Without this, qemu cannot access qcow2 backing files, see: https://bugzilla.redhat.com/497131 * src/security/security_selinux.c: re-label backing store files in SELinuxSetSecurityImageLabel() --- src/security/security_selinux.c | 28 ++++++++++++++++++++++++++++ 1 files changed, 28 insertions(+), 0 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index b84de8f..670fcb2 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -27,6 +27,7 @@ #include "logging.h" #include "pci.h" #include "hostusb.h" +#include "storage_file.h" #define VIR_FROM_THIS VIR_FROM_SECURITY @@ -403,10 +404,37 @@ SELinuxSetSecurityImageLabel(virConnectPtr conn, { const virSecurityLabelDefPtr secdef = &vm->def->seclabel; + const char *path; if (!disk->src) return 0; + path = disk->src; + do { + virStorageFileMetadata meta; + int ret; + + memset(&meta, 0, sizeof(meta)); + + ret = virStorageFileGetMetadata(conn, path, &meta); + + if (path != disk->src) + VIR_FREE(path); + path = NULL; + + if (ret < 0) + return -1; + + if (meta.backingStore != NULL && + SELinuxSetFilecon(conn, meta.backingStore, + default_content_context) < 0) { + VIR_FREE(meta.backingStore); + return -1; + } + + path = meta.backingStore; + } while (path != NULL); + if (disk->shared) { return SELinuxSetFilecon(conn, disk->src, default_image_context); } else if (disk->readonly) { -- 1.6.2.5

On Tue, Sep 29, 2009 at 09:56:49AM +0100, Mark McLoughlin wrote:
Use virStorageFileGetMetadata() to find any backing stores for images and re-label them
Without this, qemu cannot access qcow2 backing files, see:
https://bugzilla.redhat.com/497131
* src/security/security_selinux.c: re-label backing store files in SELinuxSetSecurityImageLabel() --- src/security/security_selinux.c | 28 ++++++++++++++++++++++++++++ 1 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index b84de8f..670fcb2 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -27,6 +27,7 @@ #include "logging.h" #include "pci.h" #include "hostusb.h" +#include "storage_file.h"
#define VIR_FROM_THIS VIR_FROM_SECURITY
@@ -403,10 +404,37 @@ SELinuxSetSecurityImageLabel(virConnectPtr conn,
{ const virSecurityLabelDefPtr secdef = &vm->def->seclabel; + const char *path;
if (!disk->src) return 0;
+ path = disk->src; + do { + virStorageFileMetadata meta; + int ret; + + memset(&meta, 0, sizeof(meta)); + + ret = virStorageFileGetMetadata(conn, path, &meta); + + if (path != disk->src) + VIR_FREE(path); + path = NULL; + + if (ret < 0) + return -1; + + if (meta.backingStore != NULL && + SELinuxSetFilecon(conn, meta.backingStore, + default_content_context) < 0) { + VIR_FREE(meta.backingStore); + return -1; + } + + path = meta.backingStore; + } while (path != NULL); + if (disk->shared) { return SELinuxSetFilecon(conn, disk->src, default_image_context); } else if (disk->readonly) {
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Use virStorageFileProbeHeader() to find any backing stores for images and re-label them Without this, qemu cannot access qcow2 backing files, see: https://bugzilla.redhat.com/497131 * src/security/security_selinux.c: re-label backing store files in SELinuxSetSecurityImageLabel() --- src/security/security_selinux.c | 26 ++++++++++++++++++++++++++ 1 files changed, 26 insertions(+), 0 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index b84de8f..f415118 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -27,6 +27,7 @@ #include "logging.h" #include "pci.h" #include "hostusb.h" +#include "storage_file.h" #define VIR_FROM_THIS VIR_FROM_SECURITY @@ -403,10 +404,35 @@ SELinuxSetSecurityImageLabel(virConnectPtr conn, { const virSecurityLabelDefPtr secdef = &vm->def->seclabel; + const char *path; + char *backingStore = NULL; if (!disk->src) return 0; + path = disk->src; + do { + int ret; + + ret = virStorageFileProbeHeader(conn, path, NULL, &backingStore, + NULL, NULL, NULL, NULL); + + if (path != disk->src) + VIR_FREE(path); + path = NULL; + + if (ret < 0) + return -1; + + if (backingStore != NULL && + SELinuxSetFilecon(conn, backingStore, default_content_context) < 0) { + VIR_FREE(backingStore); + return -1; + } + + path = backingStore; + } while (path != NULL); + if (disk->shared) { return SELinuxSetFilecon(conn, disk->src, default_image_context); } else if (disk->readonly) { -- 1.6.2.5

On Fri, Sep 25, 2009 at 02:27:33PM +0100, Mark McLoughlin wrote:
Use virStorageFileProbeHeader() to find any backing stores for images and re-label them
Without this, qemu cannot access qcow2 backing files, see:
https://bugzilla.redhat.com/497131
* src/security/security_selinux.c: re-label backing store files in SELinuxSetSecurityImageLabel() --- src/security/security_selinux.c | 26 ++++++++++++++++++++++++++ 1 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index b84de8f..f415118 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -27,6 +27,7 @@ #include "logging.h" #include "pci.h" #include "hostusb.h" +#include "storage_file.h"
#define VIR_FROM_THIS VIR_FROM_SECURITY
@@ -403,10 +404,35 @@ SELinuxSetSecurityImageLabel(virConnectPtr conn,
{ const virSecurityLabelDefPtr secdef = &vm->def->seclabel; + const char *path; + char *backingStore = NULL;
if (!disk->src) return 0;
+ path = disk->src; + do { + int ret; + + ret = virStorageFileProbeHeader(conn, path, NULL, &backingStore, + NULL, NULL, NULL, NULL); + + if (path != disk->src) + VIR_FREE(path); + path = NULL; + + if (ret < 0) + return -1; + + if (backingStore != NULL && + SELinuxSetFilecon(conn, backingStore, default_content_context) < 0) { + VIR_FREE(backingStore); + return -1; + } + + path = backingStore; + } while (path != NULL); + if (disk->shared) { return SELinuxSetFilecon(conn, disk->src, default_image_context); } else if (disk->readonly) {
ACK, to the principal of this patch, though obviously suggested changes earlier in the series would impact the actual code a little. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (3)
-
Daniel P. Berrange
-
Mark McLoughlin
-
Pritesh Kothari