[libvirt PATCH v2 0/9 PUSHED] cleanup storage source related code

This series is pushed, posting only to track the changes to fix the issues pointed out by Peter. Pavel Hrdina (9): util: remove unused virStorageGenerateQcowPassphrase virstoragefile: remove unused virStorageFileChainCheckBroken util: move virQEMUBuildQemuImgKeySecretOpts into storage util: move virStorageFileGetLVMKey to locking util: move virStorageFileCheckCompat into conf src: add missing headers to various files virfile: refactor virFileNBDDeviceAssociate virstoragefile: move virStorageFileResize into virfile virstoragefile: move virStorageFileIsClusterFS into virfile src/conf/storage_conf.c | 23 ++++- src/driver.c | 1 + src/libvirt_private.syms | 8 +- src/locking/lock_driver_lockd.c | 68 +++++++++++- src/lxc/lxc_controller.c | 4 +- src/qemu/qemu_interop_config.c | 1 + src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_shim.c | 1 + src/storage/parthelper.c | 1 + src/storage/storage_util.c | 76 +++++++++++++- src/util/virarptable.c | 1 + src/util/vircgroupv1.c | 1 + src/util/vircgroupv2devices.c | 1 + src/util/virfile.c | 68 ++++++++++-- src/util/virfile.h | 9 +- src/util/virpidfile.c | 1 + src/util/virqemu.c | 69 ------------- src/util/virqemu.h | 6 -- src/util/virresctrl.c | 1 + src/util/virstorageencryption.c | 34 ------ src/util/virstorageencryption.h | 2 - src/util/virstoragefile.c | 177 -------------------------------- src/util/virstoragefile.h | 12 --- src/util/virsysinfo.c | 1 + src/util/virtpm.c | 1 + tests/virstoragetest.c | 6 -- tools/virsh-console.c | 1 + tools/virsh-util.c | 1 + 28 files changed, 249 insertions(+), 328 deletions(-) -- 2.29.2

The last user was removed by commit <40f0e0348dfc84f28a500e262c4953b0d3b44fa0>. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virstorageencryption.c | 34 --------------------------------- src/util/virstorageencryption.h | 2 -- 3 files changed, 37 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 583fc5800e..d7714361d3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3118,7 +3118,6 @@ virSocketAddrSetPort; virStorageEncryptionFormat; virStorageEncryptionFree; virStorageEncryptionParseNode; -virStorageGenerateQcowPassphrase; # util/virstoragefile.h diff --git a/src/util/virstorageencryption.c b/src/util/virstorageencryption.c index a330b79637..c893f0babe 100644 --- a/src/util/virstorageencryption.c +++ b/src/util/virstorageencryption.c @@ -364,37 +364,3 @@ virStorageEncryptionFormat(virBufferPtr buf, return 0; } - -int -virStorageGenerateQcowPassphrase(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) { - virReportError(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) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot read from /dev/urandom")); - VIR_FORCE_CLOSE(fd); - return -1; - } - if (dest[i] >= 0x20 && dest[i] <= 0x7E) - i++; /* Got an acceptable character */ - } - VIR_FORCE_CLOSE(fd); - return 0; -} diff --git a/src/util/virstorageencryption.h b/src/util/virstorageencryption.h index 05a7bffdfc..352dd373d6 100644 --- a/src/util/virstorageencryption.h +++ b/src/util/virstorageencryption.h @@ -90,5 +90,3 @@ int virStorageEncryptionFormat(virBufferPtr buf, enum { VIR_STORAGE_QCOW_PASSPHRASE_SIZE = 16 }; - -int virStorageGenerateQcowPassphrase(unsigned char *dest); -- 2.29.2

The last usage outside of tests was removed by commit <780f8c94ca8b3dee7eb59c1bfbc32f672f965df8>. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virstoragefile.c | 33 --------------------------------- src/util/virstoragefile.h | 3 --- tests/virstoragetest.c | 6 ------ 4 files changed, 43 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d7714361d3..f8da4a7648 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3127,7 +3127,6 @@ virStorageAuthDefFree; virStorageAuthDefParse; virStorageFileAccess; virStorageFileCanonicalizePath; -virStorageFileChainGetBroken; virStorageFileChainLookup; virStorageFileChown; virStorageFileCreate; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index fac93118fd..119342d296 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1193,39 +1193,6 @@ virStorageFileGetMetadataFromFD(const char *path, } -/** - * virStorageFileChainCheckBroken - * - * If CHAIN is broken, set *brokenFile to the broken file name, - * otherwise set it to NULL. Caller MUST free *brokenFile after use. - * Return 0 on success (including when brokenFile is set), negative on - * error (allocation failure). - */ -int -virStorageFileChainGetBroken(virStorageSourcePtr chain, - char **brokenFile) -{ - virStorageSourcePtr tmp; - - *brokenFile = NULL; - - if (!chain) - return 0; - - for (tmp = chain; virStorageSourceIsBacking(tmp); tmp = tmp->backingStore) { - /* Break when we hit end of chain; report error if we detected - * a missing backing file, infinite loop, or other error */ - if (!tmp->backingStore && tmp->backingStoreRaw) { - *brokenFile = g_strdup(tmp->backingStoreRaw); - - return 0; - } - } - - return 0; -} - - /** * virStorageFileResize: * diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 87763cf389..cc9bd4875f 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -403,9 +403,6 @@ virStorageSourcePtr virStorageFileGetMetadataFromBuf(const char *path, size_t len, int format) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -int virStorageFileChainGetBroken(virStorageSourcePtr chain, - char **broken_file); - int virStorageFileParseChainIndex(const char *diskTarget, const char *name, unsigned int *chainIndex) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 2e466ecb99..a376154def 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -270,7 +270,6 @@ testStorageChain(const void *args) virStorageSourcePtr elt; size_t i = 0; g_autoptr(virStorageSource) meta = NULL; - g_autofree char *broken = NULL; meta = testStorageFileGetMetadata(data->start, data->format, -1, -1); if (!meta) { @@ -289,11 +288,6 @@ testStorageChain(const void *args) return -1; } - if (virStorageFileChainGetBroken(meta, &broken) || broken) { - fprintf(stderr, "chain should not be identified as broken\n"); - return -1; - } - elt = meta; while (virStorageSourceIsBacking(elt)) { g_autofree char *expect = NULL; -- 2.29.2

Function virQEMUBuildQemuImgKeySecretOpts is not used anywhere else so there is no need to have it in util. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 - src/storage/storage_util.c | 74 ++++++++++++++++++++++++++++++++++++-- src/util/virqemu.c | 69 ----------------------------------- src/util/virqemu.h | 6 ---- 4 files changed, 72 insertions(+), 78 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f8da4a7648..b8fedc82d5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2954,7 +2954,6 @@ virQEMUBuildCommandLineJSONArrayNumbered; virQEMUBuildDriveCommandlineFromJSON; virQEMUBuildNetdevCommandlineFromJSON; virQEMUBuildObjectCommandlineFromJSON; -virQEMUBuildQemuImgKeySecretOpts; # util/virrandom.h diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 2d46452da5..83b120924b 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -679,6 +679,74 @@ struct _virStorageBackendQemuImgInfo { }; +/** + * storageBackendBuildQemuImgEncriptionOpts: + * @buf: buffer to build the string into + * @encinfo: pointer to encryption info + * @alias: alias to use + * + * Generate the string for id=$alias and any encryption options for + * into the buffer. + * + * Important note, a trailing comma (",") is built into the return since + * it's expected other arguments are appended after the id=$alias string. + * So either turn something like: + * + * "key-secret=$alias," + * + * or + * "key-secret=$alias,cipher-alg=twofish-256,cipher-mode=cbc, + * hash-alg=sha256,ivgen-alg=plain64,igven-hash-alg=sha256," + * + */ +static void +storageBackendBuildQemuImgEncriptionOpts(virBufferPtr buf, + int format, + virStorageEncryptionInfoDefPtr encinfo, + const char *alias) +{ + const char *encprefix; + + if (format == VIR_STORAGE_FILE_QCOW2) { + virBufferAddLit(buf, "encrypt.format=luks,"); + encprefix = "encrypt."; + } else { + encprefix = ""; + } + + virBufferAsprintf(buf, "%skey-secret=%s,", encprefix, alias); + + if (!encinfo->cipher_name) + return; + + virBufferAsprintf(buf, "%scipher-alg=", encprefix); + virQEMUBuildBufferEscapeComma(buf, encinfo->cipher_name); + virBufferAsprintf(buf, "-%u,", encinfo->cipher_size); + if (encinfo->cipher_mode) { + virBufferAsprintf(buf, "%scipher-mode=", encprefix); + virQEMUBuildBufferEscapeComma(buf, encinfo->cipher_mode); + virBufferAddLit(buf, ","); + } + if (encinfo->cipher_hash) { + virBufferAsprintf(buf, "%shash-alg=", encprefix); + virQEMUBuildBufferEscapeComma(buf, encinfo->cipher_hash); + virBufferAddLit(buf, ","); + } + if (!encinfo->ivgen_name) + return; + + virBufferAsprintf(buf, "%sivgen-alg=", encprefix); + virQEMUBuildBufferEscapeComma(buf, encinfo->ivgen_name); + virBufferAddLit(buf, ","); + + if (encinfo->ivgen_hash) { + virBufferAsprintf(buf, "%sivgen-hash-alg=", encprefix); + virQEMUBuildBufferEscapeComma(buf, encinfo->ivgen_hash); + virBufferAddLit(buf, ","); + } +} + + static int storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr encinfo, char **opts, @@ -690,8 +758,10 @@ storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr encinfo, virBufferAsprintf(&buf, "backing_fmt=%s,", virStorageFileFormatTypeToString(info->backingFormat)); - if (encinfo) - virQEMUBuildQemuImgKeySecretOpts(&buf, info->format, encinfo, info->secretAlias); + if (encinfo) { + storageBackendBuildQemuImgEncriptionOpts(&buf, info->format, encinfo, + info->secretAlias); + } if (info->preallocate) { if (info->size_arg > info->allocation) diff --git a/src/util/virqemu.c b/src/util/virqemu.c index 5405c9eac9..c4b6e8b3db 100644 --- a/src/util/virqemu.c +++ b/src/util/virqemu.c @@ -28,7 +28,6 @@ #include "virqemu.h" #include "virstring.h" #include "viralloc.h" -#include "virstoragefile.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -384,71 +383,3 @@ virQEMUBuildBufferEscapeComma(virBufferPtr buf, const char *str) { virBufferEscape(buf, ',', ",", "%s", str); } - - -/** - * virQEMUBuildQemuImgKeySecretOpts: - * @buf: buffer to build the string into - * @encinfo: pointer to encryption info - * @alias: alias to use - * - * Generate the string for id=$alias and any encryption options for - * into the buffer. - * - * Important note, a trailing comma (",") is built into the return since - * it's expected other arguments are appended after the id=$alias string. - * So either turn something like: - * - * "key-secret=$alias," - * - * or - * "key-secret=$alias,cipher-alg=twofish-256,cipher-mode=cbc, - * hash-alg=sha256,ivgen-alg=plain64,igven-hash-alg=sha256," - * - */ -void -virQEMUBuildQemuImgKeySecretOpts(virBufferPtr buf, - int format, - virStorageEncryptionInfoDefPtr encinfo, - const char *alias) -{ - const char *encprefix; - - if (format == VIR_STORAGE_FILE_QCOW2) { - virBufferAddLit(buf, "encrypt.format=luks,"); - encprefix = "encrypt."; - } else { - encprefix = ""; - } - - virBufferAsprintf(buf, "%skey-secret=%s,", encprefix, alias); - - if (!encinfo->cipher_name) - return; - - virBufferAsprintf(buf, "%scipher-alg=", encprefix); - virQEMUBuildBufferEscapeComma(buf, encinfo->cipher_name); - virBufferAsprintf(buf, "-%u,", encinfo->cipher_size); - if (encinfo->cipher_mode) { - virBufferAsprintf(buf, "%scipher-mode=", encprefix); - virQEMUBuildBufferEscapeComma(buf, encinfo->cipher_mode); - virBufferAddLit(buf, ","); - } - if (encinfo->cipher_hash) { - virBufferAsprintf(buf, "%shash-alg=", encprefix); - virQEMUBuildBufferEscapeComma(buf, encinfo->cipher_hash); - virBufferAddLit(buf, ","); - } - if (!encinfo->ivgen_name) - return; - - virBufferAsprintf(buf, "%sivgen-alg=", encprefix); - virQEMUBuildBufferEscapeComma(buf, encinfo->ivgen_name); - virBufferAddLit(buf, ","); - - if (encinfo->ivgen_hash) { - virBufferAsprintf(buf, "%sivgen-hash-alg=", encprefix); - virQEMUBuildBufferEscapeComma(buf, encinfo->ivgen_hash); - virBufferAddLit(buf, ","); - } -} diff --git a/src/util/virqemu.h b/src/util/virqemu.h index 2b33968158..b81efc765f 100644 --- a/src/util/virqemu.h +++ b/src/util/virqemu.h @@ -25,7 +25,6 @@ #include "internal.h" #include "virbuffer.h" #include "virjson.h" -#include "virstorageencryption.h" typedef int (*virQEMUBuildCommandLineJSONArrayFormatFunc)(const char *key, virJSONValuePtr array, @@ -59,8 +58,3 @@ int virQEMUBuildObjectCommandlineFromJSON(virBufferPtr buf, char *virQEMUBuildDriveCommandlineFromJSON(virJSONValuePtr src); void virQEMUBuildBufferEscapeComma(virBufferPtr buf, const char *str); -void virQEMUBuildQemuImgKeySecretOpts(virBufferPtr buf, - int format, - virStorageEncryptionInfoDefPtr enc, - const char *alias) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); -- 2.29.2

The function doesn't take virStorageSource as argument and has nothing in common with virStorageSource or storage file. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 - src/locking/lock_driver_lockd.c | 68 ++++++++++++++++++++++++++++++++- src/util/virstoragefile.c | 62 ------------------------------ src/util/virstoragefile.h | 2 - 4 files changed, 67 insertions(+), 66 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b8fedc82d5..78d4e5ab30 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3135,7 +3135,6 @@ virStorageFileFeatureTypeToString; virStorageFileFormatTypeFromString; virStorageFileFormatTypeToString; virStorageFileGetBackingStoreStr; -virStorageFileGetLVMKey; virStorageFileGetMetadata; virStorageFileGetMetadataFromBuf; virStorageFileGetMetadataFromFD; diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index 15d9e5f076..d459c1668c 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -24,6 +24,7 @@ #include "lock_driver.h" #include "virconf.h" #include "viralloc.h" +#include "vircommand.h" #include "vircrypto.h" #include "virlog.h" #include "viruuid.h" @@ -455,6 +456,71 @@ static int virLockManagerLockDaemonNew(virLockManagerPtr lock, } +#ifdef LVS +static int +virLockManagerGetLVMKey(const char *path, + char **key) +{ + /* + * # lvs --noheadings --unbuffered --nosuffix --options "uuid" LVNAME + * 06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky + */ + int status; + int ret = -1; + g_autoptr(virCommand) cmd = NULL; + + cmd = virCommandNewArgList(LVS, "--noheadings", + "--unbuffered", "--nosuffix", + "--options", "uuid", path, + NULL + ); + *key = NULL; + + /* Run the program and capture its output */ + virCommandSetOutputBuffer(cmd, key); + if (virCommandRun(cmd, &status) < 0) + goto cleanup; + + /* Explicitly check status == 0, rather than passing NULL + * to virCommandRun because we don't want to raise an actual + * error in this scenario, just return a NULL key. + */ + + if (status == 0 && *key) { + char *nl; + char *tmp = *key; + + /* Find first non-space character */ + while (*tmp && g_ascii_isspace(*tmp)) + tmp++; + /* Kill leading spaces */ + if (tmp != *key) + memmove(*key, tmp, strlen(tmp)+1); + + /* Kill trailing newline */ + if ((nl = strchr(*key, '\n'))) + *nl = '\0'; + } + + ret = 0; + + cleanup: + if (*key && STREQ(*key, "")) + VIR_FREE(*key); + + return ret; +} +#else +static int +virLockManagerGetLVMKey(const char *path, + char **key G_GNUC_UNUSED) +{ + virReportSystemError(ENOSYS, _("Unable to get LVM key for %s"), path); + return -1; +} +#endif + + static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock, unsigned int type, const char *name, @@ -494,7 +560,7 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock, if (STRPREFIX(name, "/dev") && driver->lvmLockSpaceDir) { VIR_DEBUG("Trying to find an LVM UUID for %s", name); - if (virStorageFileGetLVMKey(name, &newName) < 0) + if (virLockManagerGetLVMKey(name, &newName) < 0) goto cleanup; if (newName) { diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 119342d296..bc342cabe3 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1251,68 +1251,6 @@ int virStorageFileIsClusterFS(const char *path) VIR_FILE_SHFS_CEPH); } -#ifdef LVS -int virStorageFileGetLVMKey(const char *path, - char **key) -{ - /* - * # lvs --noheadings --unbuffered --nosuffix --options "uuid" LVNAME - * 06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky - */ - int status; - int ret = -1; - g_autoptr(virCommand) cmd = NULL; - - cmd = virCommandNewArgList(LVS, "--noheadings", - "--unbuffered", "--nosuffix", - "--options", "uuid", path, - NULL - ); - *key = NULL; - - /* Run the program and capture its output */ - virCommandSetOutputBuffer(cmd, key); - if (virCommandRun(cmd, &status) < 0) - goto cleanup; - - /* Explicitly check status == 0, rather than passing NULL - * to virCommandRun because we don't want to raise an actual - * error in this scenario, just return a NULL key. - */ - - if (status == 0 && *key) { - char *nl; - char *tmp = *key; - - /* Find first non-space character */ - while (*tmp && g_ascii_isspace(*tmp)) - tmp++; - /* Kill leading spaces */ - if (tmp != *key) - memmove(*key, tmp, strlen(tmp)+1); - - /* Kill trailing newline */ - if ((nl = strchr(*key, '\n'))) - *nl = '\0'; - } - - ret = 0; - - cleanup: - if (*key && STREQ(*key, "")) - VIR_FREE(*key); - - return ret; -} -#else -int virStorageFileGetLVMKey(const char *path, - char **key G_GNUC_UNUSED) -{ - virReportSystemError(ENOSYS, _("Unable to get LVM key for %s"), path); - return -1; -} -#endif - #ifdef WITH_UDEV /* virStorageFileGetSCSIKey * @path: Path to the SCSI device diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index cc9bd4875f..31feb22f26 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -428,8 +428,6 @@ int virStorageFileIsClusterFS(const char *path); bool virStorageIsFile(const char *path); bool virStorageIsRelative(const char *backing); -int virStorageFileGetLVMKey(const char *path, - char **key); int virStorageFileGetSCSIKey(const char *path, char **key, bool ignoreError); -- 2.29.2

It is not used anywhere else. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/storage_conf.c | 23 ++++++++++++++++++++++- src/util/virstoragefile.c | 24 ------------------------ src/util/virstoragefile.h | 2 -- 3 files changed, 22 insertions(+), 27 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 09769e6c79..0c50529ace 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1296,6 +1296,27 @@ virStorageSize(const char *unit, } +static int +virStorageCheckCompat(const char *compat) +{ + unsigned int result; + g_auto(GStrv) version = NULL; + + if (!compat) + return 0; + + version = virStringSplit(compat, ".", 2); + if (!version || !version[1] || + virStrToLong_ui(version[0], NULL, 10, &result) < 0 || + virStrToLong_ui(version[1], NULL, 10, &result) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("forbidden characters in 'compat' attribute")); + return -1; + } + return 0; +} + + static virStorageVolDefPtr virStorageVolDefParseXML(virStoragePoolDefPtr pool, xmlXPathContextPtr ctxt, @@ -1424,7 +1445,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, } def->target.compat = virXPathString("string(./target/compat)", ctxt); - if (virStorageFileCheckCompat(def->target.compat) < 0) + if (virStorageCheckCompat(def->target.compat) < 0) return NULL; if (virXPathNode("./target/nocow", ctxt)) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index bc342cabe3..d6ff47e4a7 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -4410,30 +4410,6 @@ virStorageFileGetRelativeBackingPath(virStorageSourcePtr top, } -/* - * virStorageFileCheckCompat - */ -int -virStorageFileCheckCompat(const char *compat) -{ - unsigned int result; - g_auto(GStrv) version = NULL; - - if (!compat) - return 0; - - version = virStringSplit(compat, ".", 2); - if (!version || !version[1] || - virStrToLong_ui(version[0], NULL, 10, &result) < 0 || - virStrToLong_ui(version[1], NULL, 10, &result) < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("forbidden characters in 'compat' attribute")); - return -1; - } - return 0; -} - - /** * virStorageSourceIsRelative: * @src: storage source to check diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 31feb22f26..ec34f2d899 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -513,8 +513,6 @@ int virStorageFileGetRelativeBackingPath(virStorageSourcePtr from, char **relpath) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); -int virStorageFileCheckCompat(const char *compat); - int virStorageSourceNewFromBackingAbsolute(const char *path, virStorageSourcePtr *src); -- 2.29.2

All these headers are indirectly included provided by virfile.h having virstoragefile.h which will be removed in the following patch. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/driver.c | 1 + src/qemu/qemu_interop_config.c | 1 + src/qemu/qemu_shim.c | 1 + src/storage/parthelper.c | 1 + src/util/virarptable.c | 1 + src/util/vircgroupv1.c | 1 + src/util/vircgroupv2devices.c | 1 + src/util/virfile.h | 1 + src/util/virpidfile.c | 1 + src/util/virresctrl.c | 1 + src/util/virsysinfo.c | 1 + src/util/virtpm.c | 1 + tools/virsh-console.c | 1 + tools/virsh-util.c | 1 + 14 files changed, 14 insertions(+) diff --git a/src/driver.c b/src/driver.c index 1cacec24ff..e005a89d57 100644 --- a/src/driver.c +++ b/src/driver.c @@ -29,6 +29,7 @@ #include "virfile.h" #include "virlog.h" #include "virmodule.h" +#include "virobject.h" #include "virstring.h" #include "virthread.h" #include "virutil.h" diff --git a/src/qemu/qemu_interop_config.c b/src/qemu/qemu_interop_config.c index c5a5a9c6ec..bcaddda446 100644 --- a/src/qemu/qemu_interop_config.c +++ b/src/qemu/qemu_interop_config.c @@ -24,6 +24,7 @@ #include "configmake.h" #include "viralloc.h" #include "virenum.h" +#include "virerror.h" #include "virfile.h" #include "virhash.h" #include "virlog.h" diff --git a/src/qemu/qemu_shim.c b/src/qemu/qemu_shim.c index ef0ba086b5..18bdc99256 100644 --- a/src/qemu/qemu_shim.c +++ b/src/qemu/qemu_shim.c @@ -27,6 +27,7 @@ #include "virfile.h" #include "virstring.h" #include "virgettext.h" +#include "virthread.h" #define VIR_FROM_THIS VIR_FROM_QEMU diff --git a/src/storage/parthelper.c b/src/storage/parthelper.c index caa2e8fa62..94d588615f 100644 --- a/src/storage/parthelper.c +++ b/src/storage/parthelper.c @@ -40,6 +40,7 @@ #include "virstring.h" #include "virgettext.h" #include "virdevmapper.h" +#include "virerror.h" /* we don't need to include the full internal.h just for this */ #define STREQ(a, b) (strcmp(a, b) == 0) diff --git a/src/util/virarptable.c b/src/util/virarptable.c index 01a27c0093..d62de5e3dd 100644 --- a/src/util/virarptable.c +++ b/src/util/virarptable.c @@ -26,6 +26,7 @@ #include "viralloc.h" #include "virarptable.h" +#include "virerror.h" #include "virfile.h" #include "virlog.h" #include "virnetlink.h" diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 06849efd38..2b4d625c35 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -41,6 +41,7 @@ #include "virsystemd.h" #include "virerror.h" #include "viralloc.h" +#include "virthread.h" VIR_LOG_INIT("util.cgroup"); diff --git a/src/util/vircgroupv2devices.c b/src/util/vircgroupv2devices.c index f31244f3c1..71591be4c4 100644 --- a/src/util/vircgroupv2devices.c +++ b/src/util/vircgroupv2devices.c @@ -34,6 +34,7 @@ #include "virbpf.h" #include "vircgroup.h" #include "vircgroupv2devices.h" +#include "virerror.h" #include "virfile.h" #include "virlog.h" diff --git a/src/util/virfile.h b/src/util/virfile.h index 32505826ee..0f197be73d 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -29,6 +29,7 @@ #include "internal.h" #include "virbitmap.h" #include "virstoragefile.h" +#include "virenum.h" typedef enum { VIR_FILE_CLOSE_PRESERVE_ERRNO = 1 << 0, diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c index 895eb83ea9..b4d0059405 100644 --- a/src/util/virpidfile.c +++ b/src/util/virpidfile.c @@ -30,6 +30,7 @@ #include "virpidfile.h" #include "virfile.h" #include "viralloc.h" +#include "virbuffer.h" #include "virutil.h" #include "virlog.h" #include "virerror.h" diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index d3087b98c1..67a921b44b 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -27,6 +27,7 @@ #define LIBVIRT_VIRRESCTRLPRIV_H_ALLOW #include "virresctrlpriv.h" #include "viralloc.h" +#include "virbuffer.h" #include "virfile.h" #include "virlog.h" #include "virobject.h" diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 217f842a37..28f7104145 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -32,6 +32,7 @@ #include "virlog.h" #include "virfile.h" #include "virstring.h" +#include "virxml.h" #define LIBVIRT_VIRSYSINFOPRIV_H_ALLOW #include "virsysinfopriv.h" diff --git a/src/util/virtpm.c b/src/util/virtpm.c index 0e11674a3c..b41eb00619 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -31,6 +31,7 @@ #include "virbitmap.h" #include "virjson.h" #include "virlog.h" +#include "virthread.h" #define VIR_FROM_THIS VIR_FROM_TPM diff --git a/tools/virsh-console.c b/tools/virsh-console.c index 2e498a6903..4e9bdb67e1 100644 --- a/tools/virsh-console.c +++ b/tools/virsh-console.c @@ -38,6 +38,7 @@ # include "viralloc.h" # include "virthread.h" # include "virerror.h" +# include "virobject.h" VIR_LOG_INIT("tools.virsh-console"); diff --git a/tools/virsh-util.c b/tools/virsh-util.c index af7ed55348..9f69bad8cc 100644 --- a/tools/virsh-util.c +++ b/tools/virsh-util.c @@ -23,6 +23,7 @@ #include "virfile.h" #include "virstring.h" #include "viralloc.h" +#include "virxml.h" static virDomainPtr virshLookupDomainInternal(vshControl *ctl, -- 2.29.2

The only reason why virstoragefile.h needs to be included in virfile.h is that virFileNBDDeviceAssociate() takes virStorageFileFormat argument. The function doesn't need the enum value as it converts the value to string and uses only that. Change the argument to string which will allow us to remove that include. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/lxc/lxc_controller.c | 4 ++-- src/util/virfile.c | 8 ++------ src/util/virfile.h | 3 +-- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 67e5e63d00..8f166a436a 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -509,7 +509,7 @@ static int virLXCControllerSetupNBDDeviceFS(virDomainFSDefPtr fs) } if (virFileNBDDeviceAssociate(fs->src->path, - fs->format, + virStorageFileFormatTypeToString(fs->format), fs->readonly, &dev) < 0) return -1; @@ -541,7 +541,7 @@ static int virLXCControllerSetupNBDDeviceDisk(virDomainDiskDefPtr disk) } if (virFileNBDDeviceAssociate(src, - format, + virStorageFileFormatTypeToString(format), disk->src->readonly, &dev) < 0) return -1; diff --git a/src/util/virfile.c b/src/util/virfile.c index f7283fa72f..3f4c6d1d0a 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -879,14 +879,13 @@ virFileNBDLoadDriver(void) } int virFileNBDDeviceAssociate(const char *file, - virStorageFileFormat fmt, + const char *fmtstr, bool readonly, char **dev) { g_autofree char *nbddev = NULL; g_autofree char *qemunbd = NULL; g_autoptr(virCommand) cmd = NULL; - const char *fmtstr = NULL; if (!virFileNBDLoadDriver()) return -1; @@ -900,9 +899,6 @@ int virFileNBDDeviceAssociate(const char *file, return -1; } - if (fmt > 0) - fmtstr = virStorageFileFormatTypeToString(fmt); - cmd = virCommandNew(qemunbd); /* Explicitly not trying to cope with old qemu-nbd which @@ -945,7 +941,7 @@ int virFileLoopDeviceAssociate(const char *file, } int virFileNBDDeviceAssociate(const char *file, - virStorageFileFormat fmt G_GNUC_UNUSED, + const char *fmtstr G_GNUC_UNUSED, bool readonly G_GNUC_UNUSED, char **dev G_GNUC_UNUSED) { diff --git a/src/util/virfile.h b/src/util/virfile.h index 0f197be73d..28dfe86445 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -28,7 +28,6 @@ #include "internal.h" #include "virbitmap.h" -#include "virstoragefile.h" #include "virenum.h" typedef enum { @@ -144,7 +143,7 @@ int virFileLoopDeviceAssociate(const char *file, char **dev); int virFileNBDDeviceAssociate(const char *file, - virStorageFileFormat fmt, + const char *fmtstr, bool readonly, char **dev); -- 2.29.2

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 2 +- src/storage/storage_util.c | 2 +- src/util/virfile.c | 47 ++++++++++++++++++++++++++++++++++++++ src/util/virfile.h | 4 ++++ src/util/virstoragefile.c | 47 -------------------------------------- src/util/virstoragefile.h | 4 ---- 6 files changed, 53 insertions(+), 53 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 78d4e5ab30..e79dc54d33 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2121,6 +2121,7 @@ virFileRelLinkPointsTo; virFileRemove; virFileRemoveLastComponent; virFileRemoveXAttr; +virFileResize; virFileResolveAllLinks; virFileResolveLink; virFileRewrite; @@ -3150,7 +3151,6 @@ virStorageFileParseChainIndex; virStorageFileProbeFormat; virStorageFileRead; virStorageFileReportBrokenChain; -virStorageFileResize; virStorageFileStat; virStorageFileSupportsAccess; virStorageFileSupportsBackingChainTraversal; diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 83b120924b..2478cb0a4a 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -2394,7 +2394,7 @@ virStorageBackendVolResizeLocal(virStoragePoolObjPtr pool, VIR_STORAGE_VOL_RESIZE_SHRINK, -1); if (vol->target.format == VIR_STORAGE_FILE_RAW && !vol->target.encryption) { - return virStorageFileResize(vol->target.path, capacity, pre_allocate); + return virFileResize(vol->target.path, capacity, pre_allocate); } else if (vol->target.format == VIR_STORAGE_FILE_RAW && vol->target.encryption) { if (pre_allocate) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", diff --git a/src/util/virfile.c b/src/util/virfile.c index 3f4c6d1d0a..6e16780e30 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -557,6 +557,53 @@ virFileRewriteStr(const char *path, } +/** + * virFileResize: + * + * Change the capacity of the raw storage file at 'path'. + */ +int +virFileResize(const char *path, + unsigned long long capacity, + bool pre_allocate) +{ + int rc; + VIR_AUTOCLOSE fd = -1; + + if ((fd = open(path, O_RDWR)) < 0) { + virReportSystemError(errno, _("Unable to open '%s'"), path); + return -1; + } + + if (pre_allocate) { + if ((rc = virFileAllocate(fd, 0, capacity)) != 0) { + if (rc == -2) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("preallocate is not supported on this platform")); + } else { + virReportSystemError(errno, + _("Failed to pre-allocate space for " + "file '%s'"), path); + } + return -1; + } + } + + if (ftruncate(fd, capacity) < 0) { + virReportSystemError(errno, + _("Failed to truncate file '%s'"), path); + return -1; + } + + if (VIR_CLOSE(fd) < 0) { + virReportSystemError(errno, _("Unable to save '%s'"), path); + return -1; + } + + return 0; +} + + int virFileTouch(const char *path, mode_t mode) { int fd = -1; diff --git a/src/util/virfile.h b/src/util/virfile.h index 28dfe86445..45b75a059e 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -133,6 +133,10 @@ int virFileRewriteStr(const char *path, mode_t mode, const char *str); +int virFileResize(const char *path, + unsigned long long capacity, + bool pre_allocate); + int virFileTouch(const char *path, mode_t mode); int virFileUpdatePerm(const char *path, diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index d6ff47e4a7..66694bfcc0 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1193,53 +1193,6 @@ virStorageFileGetMetadataFromFD(const char *path, } -/** - * virStorageFileResize: - * - * Change the capacity of the raw storage file at 'path'. - */ -int -virStorageFileResize(const char *path, - unsigned long long capacity, - bool pre_allocate) -{ - int rc; - VIR_AUTOCLOSE fd = -1; - - if ((fd = open(path, O_RDWR)) < 0) { - virReportSystemError(errno, _("Unable to open '%s'"), path); - return -1; - } - - if (pre_allocate) { - if ((rc = virFileAllocate(fd, 0, capacity)) != 0) { - if (rc == -2) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("preallocate is not supported on this platform")); - } else { - virReportSystemError(errno, - _("Failed to pre-allocate space for " - "file '%s'"), path); - } - return -1; - } - } - - if (ftruncate(fd, capacity) < 0) { - virReportSystemError(errno, - _("Failed to truncate file '%s'"), path); - return -1; - } - - if (VIR_CLOSE(fd) < 0) { - virReportSystemError(errno, _("Unable to save '%s'"), path); - return -1; - } - - return 0; -} - - int virStorageFileIsClusterFS(const char *path) { /* These are coherent cluster filesystems known to be safe for diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index ec34f2d899..94bb889d84 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -420,10 +420,6 @@ virStorageSourcePtr virStorageFileChainLookup(virStorageSourcePtr chain, virStorageSourcePtr *parent) ATTRIBUTE_NONNULL(1); -int virStorageFileResize(const char *path, - unsigned long long capacity, - bool pre_allocate); - int virStorageFileIsClusterFS(const char *path); bool virStorageIsFile(const char *path); bool virStorageIsRelative(const char *backing); -- 2.29.2

Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 2 +- src/qemu/qemu_migration.c | 2 +- src/util/virfile.c | 13 +++++++++++++ src/util/virfile.h | 1 + src/util/virstoragefile.c | 11 ----------- src/util/virstoragefile.h | 1 - 6 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e79dc54d33..c67bd2504a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2087,6 +2087,7 @@ virFileGetXAttr; virFileGetXAttrQuiet; virFileInData; virFileIsCDROM; +virFileIsClusterFS; virFileIsDir; virFileIsExecutable; virFileIsLink; @@ -3145,7 +3146,6 @@ virStorageFileGetSCSIKey; virStorageFileGetUniqueIdentifier; virStorageFileInit; virStorageFileInitAs; -virStorageFileIsClusterFS; virStorageFileParseBackingStoreStr; virStorageFileParseChainIndex; virStorageFileProbeFormat; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 90b0ec95e3..07235d6480 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1466,7 +1466,7 @@ qemuMigrationSrcIsSafe(virDomainDefPtr def, } else if (rc == 0) { unsafe = true; } - if ((rc = virStorageFileIsClusterFS(src)) < 0) + if ((rc = virFileIsClusterFS(src)) < 0) return false; else if (rc == 1) continue; diff --git a/src/util/virfile.c b/src/util/virfile.c index 6e16780e30..609cafdd34 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -3709,6 +3709,19 @@ int virFileIsSharedFS(const char *path) } +int +virFileIsClusterFS(const char *path) +{ + /* These are coherent cluster filesystems known to be safe for + * migration with cache != none + */ + return virFileIsSharedFSType(path, + VIR_FILE_SHFS_GFS2 | + VIR_FILE_SHFS_OCFS | + VIR_FILE_SHFS_CEPH); +} + + #if defined(__linux__) && defined(WITH_SYS_MOUNT_H) int virFileSetupDev(const char *path, diff --git a/src/util/virfile.h b/src/util/virfile.h index 45b75a059e..733d652ac9 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -223,6 +223,7 @@ enum { int virFileIsSharedFSType(const char *path, int fstypes) ATTRIBUTE_NONNULL(1); int virFileIsSharedFS(const char *path) ATTRIBUTE_NONNULL(1); +int virFileIsClusterFS(const char *path) ATTRIBUTE_NONNULL(1); int virFileIsMountPoint(const char *file) ATTRIBUTE_NONNULL(1); int virFileIsCDROM(const char *path) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 66694bfcc0..3db85d8b89 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1193,17 +1193,6 @@ virStorageFileGetMetadataFromFD(const char *path, } -int virStorageFileIsClusterFS(const char *path) -{ - /* These are coherent cluster filesystems known to be safe for - * migration with cache != none - */ - return virFileIsSharedFSType(path, - VIR_FILE_SHFS_GFS2 | - VIR_FILE_SHFS_OCFS | - VIR_FILE_SHFS_CEPH); -} - #ifdef WITH_UDEV /* virStorageFileGetSCSIKey * @path: Path to the SCSI device diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 94bb889d84..2452b967b2 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -420,7 +420,6 @@ virStorageSourcePtr virStorageFileChainLookup(virStorageSourcePtr chain, virStorageSourcePtr *parent) ATTRIBUTE_NONNULL(1); -int virStorageFileIsClusterFS(const char *path); bool virStorageIsFile(const char *path); bool virStorageIsRelative(const char *backing); -- 2.29.2
participants (1)
-
Pavel Hrdina