[libvirt] [PATCH v2 0/4] Fix VolOpen error reporting

In https://bugzilla.redhat.com/show_bug.cgi?id=1080613 , something is causing StorageVolGetXML to fail, but it's not raising any error message. Code inspection found that most users of VolOpen are susceptible to error reporting issues. The first 3 patches are some code reorganization and cleanups that make it easier to identify the VolOpen callers. Patch 4 fixes up the reporting issue. v2: Jan's suggestion to only report an error if caller passed in VOL_OPEN_ERROR Cole Robinson (4): storage: Rename UpdateVolInfoFlags to UpdateVolInfo storage: move block format lookup to shared UpdateVolInfo storage: Rename VolOpenCheckMode to VolOpen storage: Report error from VolOpen if proper flag is passed src/storage/storage_backend.c | 260 ++++++++++++++++++---------------- src/storage/storage_backend.h | 18 +-- src/storage/storage_backend_disk.c | 3 +- src/storage/storage_backend_fs.c | 25 ++-- src/storage/storage_backend_logical.c | 11 +- src/storage/storage_backend_mpath.c | 36 +---- src/storage/storage_backend_scsi.c | 43 +----- 7 files changed, 175 insertions(+), 221 deletions(-) -- 1.8.5.3

And drop the original UpdateVolInfo. Makes it a bit easier to follow the function usage. And change the int parameter to an explicit bool. --- src/storage/storage_backend.c | 13 +++---------- src/storage/storage_backend.h | 6 ++---- src/storage/storage_backend_disk.c | 3 ++- src/storage/storage_backend_fs.c | 4 ++-- src/storage/storage_backend_logical.c | 3 ++- 5 files changed, 11 insertions(+), 18 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 5b3b536..7795b33 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1338,9 +1338,9 @@ virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target, } int -virStorageBackendUpdateVolInfoFlags(virStorageVolDefPtr vol, - int withCapacity, - unsigned int openflags) +virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, + bool withCapacity, + unsigned int openflags) { int ret; @@ -1359,13 +1359,6 @@ virStorageBackendUpdateVolInfoFlags(virStorageVolDefPtr vol, return 0; } -int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, - int withCapacity) -{ - return virStorageBackendUpdateVolInfoFlags(vol, withCapacity, - VIR_STORAGE_VOL_OPEN_DEFAULT); -} - /* * virStorageBackendUpdateVolTargetInfoFD: * @target: target definition ptr of volume to update diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 2034a22..56f8d03 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -138,10 +138,8 @@ int virStorageBackendVolOpenCheckMode(const char *path, struct stat *sb, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, - int withCapacity); -int virStorageBackendUpdateVolInfoFlags(virStorageVolDefPtr vol, - int withCapacity, - unsigned int openflags); + bool withCapacity, + unsigned int openflags); int virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target, unsigned long long *allocation, unsigned long long *capacity, diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 8276c96..a8652c1 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -113,7 +113,8 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool, } /* Refresh allocation/capacity/perms */ - if (virStorageBackendUpdateVolInfo(vol, 1) < 0) + if (virStorageBackendUpdateVolInfo(vol, true, + VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) return -1; /* set partition type */ diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 0f8da06..aa3ad2b 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1180,8 +1180,8 @@ virStorageBackendFileSystemVolRefresh(virConnectPtr conn, int ret; /* Refresh allocation / permissions info in case its changed */ - ret = virStorageBackendUpdateVolInfoFlags(vol, 0, - VIR_STORAGE_VOL_FS_OPEN_FLAGS); + ret = virStorageBackendUpdateVolInfo(vol, false, + VIR_STORAGE_VOL_FS_OPEN_FLAGS); if (ret < 0) return ret; diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index f90d373..f2254a4 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -149,7 +149,8 @@ virStorageBackendLogicalMakeVol(char **const groups, if (!vol->key && VIR_STRDUP(vol->key, groups[2]) < 0) goto cleanup; - if (virStorageBackendUpdateVolInfo(vol, 1) < 0) + if (virStorageBackendUpdateVolInfo(vol, true, + VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) goto cleanup; nextents = 1; -- 1.8.5.3

On 03/31/2014 12:50 PM, Cole Robinson wrote:
And drop the original UpdateVolInfo. Makes it a bit easier to follow the function usage.
And change the int parameter to an explicit bool. --- src/storage/storage_backend.c | 13 +++---------- src/storage/storage_backend.h | 6 ++---- src/storage/storage_backend_disk.c | 3 ++- src/storage/storage_backend_fs.c | 4 ++-- src/storage/storage_backend_logical.c | 3 ++- 5 files changed, 11 insertions(+), 18 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

--- src/storage/storage_backend.c | 175 ++++++++++++++++++---------------- src/storage/storage_backend.h | 4 +- src/storage/storage_backend_disk.c | 2 +- src/storage/storage_backend_fs.c | 4 +- src/storage/storage_backend_logical.c | 2 +- src/storage/storage_backend_mpath.c | 36 +------ src/storage/storage_backend_scsi.c | 39 +------- 7 files changed, 103 insertions(+), 159 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 7795b33..78644f6 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1198,6 +1198,80 @@ virStorageFileBackendForType(int type, } +struct diskType { + int part_table_type; + unsigned short offset; + unsigned short length; + unsigned long long magic; +}; + + +static struct diskType const disk_types[] = { + { VIR_STORAGE_POOL_DISK_LVM2, 0x218, 8, 0x31303020324D564CULL }, + { VIR_STORAGE_POOL_DISK_GPT, 0x200, 8, 0x5452415020494645ULL }, + { VIR_STORAGE_POOL_DISK_DVH, 0x0, 4, 0x41A9E50BULL }, + { VIR_STORAGE_POOL_DISK_MAC, 0x0, 2, 0x5245ULL }, + { VIR_STORAGE_POOL_DISK_BSD, 0x40, 4, 0x82564557ULL }, + { VIR_STORAGE_POOL_DISK_SUN, 0x1fc, 2, 0xBEDAULL }, + /* + * NOTE: pc98 is funky; the actual signature is 0x55AA (just like dos), so + * we can't use that. At the moment I'm relying on the "dummy" IPL + * bootloader data that comes from parted. Luckily, the chances of running + * into a pc98 machine running libvirt are approximately nil. + */ + /*{ 0x1fe, 2, 0xAA55UL },*/ + { VIR_STORAGE_POOL_DISK_PC98, 0x0, 8, 0x314C5049000000CBULL }, + /* + * NOTE: the order is important here; some other disk types (like GPT and + * and PC98) also have 0x55AA at this offset. For that reason, the DOS + * one must be the last one. + */ + { VIR_STORAGE_POOL_DISK_DOS, 0x1fe, 2, 0xAA55ULL }, + { -1, 0x0, 0, 0x0ULL }, +}; + + +static int +virStorageBackendDetectBlockVolFormatFD(virStorageVolTargetPtr target, + int fd) +{ + size_t i; + off_t start; + unsigned char buffer[1024]; + ssize_t bytes; + + /* make sure to set the target format "unknown" to begin with */ + target->format = VIR_STORAGE_POOL_DISK_UNKNOWN; + + start = lseek(fd, 0, SEEK_SET); + if (start < 0) { + virReportSystemError(errno, + _("cannot seek to beginning of file '%s'"), + target->path); + return -1; + } + bytes = saferead(fd, buffer, sizeof(buffer)); + if (bytes < 0) { + virReportSystemError(errno, + _("cannot read beginning of file '%s'"), + target->path); + return -1; + } + + for (i = 0; disk_types[i].part_table_type != -1; i++) { + if (disk_types[i].offset + disk_types[i].length > bytes) + continue; + if (memcmp(buffer+disk_types[i].offset, &disk_types[i].magic, + disk_types[i].length) == 0) { + target->format = disk_types[i].part_table_type; + break; + } + } + + return 0; +} + + /* * Allows caller to silently ignore files with improper mode * @@ -1316,22 +1390,30 @@ int virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target, unsigned long long *allocation, unsigned long long *capacity, + bool withBlockVolFormat, unsigned int openflags) { - int ret, fd; + int ret, fd = -1; struct stat sb; if ((ret = virStorageBackendVolOpenCheckMode(target->path, &sb, openflags)) < 0) - return ret; - + goto cleanup; fd = ret; - ret = virStorageBackendUpdateVolTargetInfoFD(target, - fd, - &sb, - allocation, - capacity); + if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, + fd, + &sb, + allocation, + capacity)) < 0) + goto cleanup; + + if (withBlockVolFormat) { + if ((ret = virStorageBackendDetectBlockVolFormatFD(target, fd)) < 0) + goto cleanup; + } + + cleanup: VIR_FORCE_CLOSE(fd); return ret; @@ -1340,6 +1422,7 @@ virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target, int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, bool withCapacity, + bool withBlockVolFormat, unsigned int openflags) { int ret; @@ -1347,12 +1430,14 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, if ((ret = virStorageBackendUpdateVolTargetInfo(&vol->target, &vol->allocation, withCapacity ? &vol->capacity : NULL, + withBlockVolFormat, openflags)) < 0) return ret; if (vol->backingStore.path && (ret = virStorageBackendUpdateVolTargetInfo(&vol->backingStore, NULL, NULL, + withBlockVolFormat, VIR_STORAGE_VOL_OPEN_DEFAULT)) < 0) return ret; @@ -1455,80 +1540,6 @@ virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target, } -struct diskType { - int part_table_type; - unsigned short offset; - unsigned short length; - unsigned long long magic; -}; - - -static struct diskType const disk_types[] = { - { VIR_STORAGE_POOL_DISK_LVM2, 0x218, 8, 0x31303020324D564CULL }, - { VIR_STORAGE_POOL_DISK_GPT, 0x200, 8, 0x5452415020494645ULL }, - { VIR_STORAGE_POOL_DISK_DVH, 0x0, 4, 0x41A9E50BULL }, - { VIR_STORAGE_POOL_DISK_MAC, 0x0, 2, 0x5245ULL }, - { VIR_STORAGE_POOL_DISK_BSD, 0x40, 4, 0x82564557ULL }, - { VIR_STORAGE_POOL_DISK_SUN, 0x1fc, 2, 0xBEDAULL }, - /* - * NOTE: pc98 is funky; the actual signature is 0x55AA (just like dos), so - * we can't use that. At the moment I'm relying on the "dummy" IPL - * bootloader data that comes from parted. Luckily, the chances of running - * into a pc98 machine running libvirt are approximately nil. - */ - /*{ 0x1fe, 2, 0xAA55UL },*/ - { VIR_STORAGE_POOL_DISK_PC98, 0x0, 8, 0x314C5049000000CBULL }, - /* - * NOTE: the order is important here; some other disk types (like GPT and - * and PC98) also have 0x55AA at this offset. For that reason, the DOS - * one must be the last one. - */ - { VIR_STORAGE_POOL_DISK_DOS, 0x1fe, 2, 0xAA55ULL }, - { -1, 0x0, 0, 0x0ULL }, -}; - - -int -virStorageBackendDetectBlockVolFormatFD(virStorageVolTargetPtr target, - int fd) -{ - size_t i; - off_t start; - unsigned char buffer[1024]; - ssize_t bytes; - - /* make sure to set the target format "unknown" to begin with */ - target->format = VIR_STORAGE_POOL_DISK_UNKNOWN; - - start = lseek(fd, 0, SEEK_SET); - if (start < 0) { - virReportSystemError(errno, - _("cannot seek to beginning of file '%s'"), - target->path); - return -1; - } - bytes = saferead(fd, buffer, sizeof(buffer)); - if (bytes < 0) { - virReportSystemError(errno, - _("cannot read beginning of file '%s'"), - target->path); - return -1; - } - - for (i = 0; disk_types[i].part_table_type != -1; i++) { - if (disk_types[i].offset + disk_types[i].length > bytes) - continue; - if (memcmp(buffer+disk_types[i].offset, &disk_types[i].magic, - disk_types[i].length) == 0) { - target->format = disk_types[i].part_table_type; - break; - } - } - - return 0; -} - - /* * Given a volume path directly in /dev/XXX, iterate over the * entries in the directory pool->def->target.path and find the diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 56f8d03..de32a27 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -139,18 +139,18 @@ int virStorageBackendVolOpenCheckMode(const char *path, struct stat *sb, int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, bool withCapacity, + bool withBlockVolFormat, unsigned int openflags); int virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target, unsigned long long *allocation, unsigned long long *capacity, + bool withBlockVolFormat, unsigned int openflags); int virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target, int fd, struct stat *sb, unsigned long long *allocation, unsigned long long *capacity); -int virStorageBackendDetectBlockVolFormatFD(virStorageVolTargetPtr target, - int fd); char *virStorageBackendStablePath(virStoragePoolObjPtr pool, const char *devpath, diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index a8652c1..8d09500 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -113,7 +113,7 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool, } /* Refresh allocation/capacity/perms */ - if (virStorageBackendUpdateVolInfo(vol, true, + if (virStorageBackendUpdateVolInfo(vol, true, false, VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) return -1; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index aa3ad2b..e4de498 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -895,7 +895,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, vol->backingStore.format = backingStoreFormat; if (virStorageBackendUpdateVolTargetInfo(&vol->backingStore, - NULL, NULL, + NULL, NULL, false, VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) { /* The backing file is currently unavailable, the capacity, * allocation, owner, group and mode are unknown. Just log the @@ -1180,7 +1180,7 @@ virStorageBackendFileSystemVolRefresh(virConnectPtr conn, int ret; /* Refresh allocation / permissions info in case its changed */ - ret = virStorageBackendUpdateVolInfo(vol, false, + ret = virStorageBackendUpdateVolInfo(vol, false, false, VIR_STORAGE_VOL_FS_OPEN_FLAGS); if (ret < 0) return ret; diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index f2254a4..a047a5d 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -149,7 +149,7 @@ virStorageBackendLogicalMakeVol(char **const groups, if (!vol->key && VIR_STRDUP(vol->key, groups[2]) < 0) goto cleanup; - if (virStorageBackendUpdateVolInfo(vol, true, + if (virStorageBackendUpdateVolInfo(vol, true, false, VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) goto cleanup; diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index 82e3e20..1242150 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -42,37 +42,6 @@ VIR_LOG_INIT("storage.storage_backend_mpath"); static int -virStorageBackendMpathUpdateVolTargetInfo(virStorageVolTargetPtr target, - unsigned long long *allocation, - unsigned long long *capacity) -{ - int ret = -1; - int fdret, fd = -1; - struct stat sb; - - if ((fdret = virStorageBackendVolOpenCheckMode(target->path, &sb, - VIR_STORAGE_VOL_OPEN_DEFAULT)) < 0) - goto out; - fd = fdret; - - if (virStorageBackendUpdateVolTargetInfoFD(target, - fd, - &sb, - allocation, - capacity) < 0) - goto out; - - if (virStorageBackendDetectBlockVolFormatFD(target, fd) < 0) - goto out; - - ret = 0; - out: - VIR_FORCE_CLOSE(fd); - return ret; -} - - -static int virStorageBackendMpathNewVol(virStoragePoolObjPtr pool, const int devnum, const char *dev) @@ -91,9 +60,8 @@ virStorageBackendMpathNewVol(virStoragePoolObjPtr pool, if (virAsprintf(&vol->target.path, "/dev/%s", dev) < 0) goto cleanup; - if (virStorageBackendMpathUpdateVolTargetInfo(&vol->target, - &vol->allocation, - &vol->capacity) < 0) { + if (virStorageBackendUpdateVolInfo(vol, true, true, + VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) { goto cleanup; } diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index a318f29..4c2484d 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -102,39 +102,6 @@ getDeviceType(uint32_t host, return retval; } -static int -virStorageBackendSCSIUpdateVolTargetInfo(virStorageVolTargetPtr target, - unsigned long long *allocation, - unsigned long long *capacity) -{ - int fdret, fd = -1; - int ret = -1; - struct stat sb; - - if ((fdret = virStorageBackendVolOpenCheckMode(target->path, &sb, - VIR_STORAGE_VOL_OPEN_DEFAULT)) < 0) - goto cleanup; - fd = fdret; - - if (virStorageBackendUpdateVolTargetInfoFD(target, - fd, - &sb, - allocation, - capacity) < 0) - goto cleanup; - - if (virStorageBackendDetectBlockVolFormatFD(target, fd) < 0) - goto cleanup; - - ret = 0; - - cleanup: - VIR_FORCE_CLOSE(fd); - - return ret; -} - - static char * virStorageBackendSCSISerial(const char *dev) { @@ -232,10 +199,8 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, goto free_vol; } - if (virStorageBackendSCSIUpdateVolTargetInfo(&vol->target, - &vol->allocation, - &vol->capacity) < 0) { - + if (virStorageBackendUpdateVolInfo(vol, true, true, + VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to update volume for '%s'"), devpath); -- 1.8.5.3

On 03/31/2014 12:50 PM, Cole Robinson wrote:
---
Commit message is a bit sparse - it says what but not why. But looking at the patch, it looks like the goal was to refactor code that copied and pasted block format lookup after getting volinfo to instead have volinfo get everything up front.
src/storage/storage_backend.c | 175 ++++++++++++++++++---------------- src/storage/storage_backend.h | 4 +- src/storage/storage_backend_disk.c | 2 +- src/storage/storage_backend_fs.c | 4 +- src/storage/storage_backend_logical.c | 2 +- src/storage/storage_backend_mpath.c | 36 +------ src/storage/storage_backend_scsi.c | 39 +------- 7 files changed, 103 insertions(+), 159 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 7795b33..78644f6 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1198,6 +1198,80 @@ virStorageFileBackendForType(int type, }
+struct diskType { + int part_table_type; + unsigned short offset;
So this hunk is code motion;
@@ -1316,22 +1390,30 @@ int virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target, unsigned long long *allocation, unsigned long long *capacity, + bool withBlockVolFormat,
This hunk adds a new parameter that controls the use of the consolidated code...
+++ b/src/storage/storage_backend_disk.c @@ -113,7 +113,7 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool, }
/* Refresh allocation/capacity/perms */ - if (virStorageBackendUpdateVolInfo(vol, true, + if (virStorageBackendUpdateVolInfo(vol, true, false,
...most callers don't need the block lookup
+++ b/src/storage/storage_backend_mpath.c @@ -42,37 +42,6 @@ VIR_LOG_INIT("storage.storage_backend_mpath");
static int -virStorageBackendMpathUpdateVolTargetInfo(virStorageVolTargetPtr target, - unsigned long long *allocation, - unsigned long long *capacity) -{
and those that did now get it from the common function. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Remove the original VolOpen implementation, which is now only used in one spot. --- src/storage/storage_backend.c | 14 +++----------- src/storage/storage_backend.h | 8 ++------ src/storage/storage_backend_fs.c | 2 +- src/storage/storage_backend_logical.c | 4 +++- 4 files changed, 9 insertions(+), 19 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 78644f6..42bd445 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1279,8 +1279,8 @@ virStorageBackendDetectBlockVolFormatFD(virStorageVolTargetPtr target, * volume is a dangling symbolic link. */ int -virStorageBackendVolOpenCheckMode(const char *path, struct stat *sb, - unsigned int flags) +virStorageBackendVolOpen(const char *path, struct stat *sb, + unsigned int flags) { int fd, mode = 0; char *base = last_component(path); @@ -1379,13 +1379,6 @@ virStorageBackendVolOpenCheckMode(const char *path, struct stat *sb, return fd; } -int virStorageBackendVolOpen(const char *path) -{ - struct stat sb; - return virStorageBackendVolOpenCheckMode(path, &sb, - VIR_STORAGE_VOL_OPEN_DEFAULT); -} - int virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target, unsigned long long *allocation, @@ -1396,8 +1389,7 @@ virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target, int ret, fd = -1; struct stat sb; - if ((ret = virStorageBackendVolOpenCheckMode(target->path, &sb, - openflags)) < 0) + if ((ret = virStorageBackendVolOpen(target->path, &sb, openflags)) < 0) goto cleanup; fd = ret; diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index de32a27..9b8ef7f 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -114,10 +114,6 @@ struct _virStorageBackend { virStorageBackendPtr virStorageBackendForType(int type); -int virStorageBackendVolOpen(const char *path) -ATTRIBUTE_RETURN_CHECK -ATTRIBUTE_NONNULL(1); - /* VolOpenCheckMode flags */ enum { VIR_STORAGE_VOL_OPEN_ERROR = 1 << 0, /* warn if unexpected type @@ -132,8 +128,8 @@ enum { VIR_STORAGE_VOL_OPEN_REG |\ VIR_STORAGE_VOL_OPEN_BLOCK) -int virStorageBackendVolOpenCheckMode(const char *path, struct stat *sb, - unsigned int flags) +int virStorageBackendVolOpen(const char *path, struct stat *sb, + unsigned int flags) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index e4de498..e02d17f 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -81,7 +81,7 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target, if (encryption) *encryption = NULL; - if ((ret = virStorageBackendVolOpenCheckMode(target->path, &sb, + if ((ret = virStorageBackendVolOpen(target->path, &sb, VIR_STORAGE_VOL_FS_REFRESH_FLAGS)) < 0) goto error; /* Take care to propagate ret, it is not always -1 */ fd = ret; diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index a047a5d..7893626 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -719,6 +719,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, int fd = -1; virCommandPtr cmd = NULL; virErrorPtr err; + struct stat sb; if (vol->target.encryption != NULL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -760,7 +761,8 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, virCommandFree(cmd); cmd = NULL; - if ((fd = virStorageBackendVolOpen(vol->target.path)) < 0) + if ((fd = virStorageBackendVolOpen(vol->target.path, &sb, + VIR_STORAGE_VOL_OPEN_DEFAULT)) < 0) goto error; /* We can only chown/grp if root */ -- 1.8.5.3

On 03/31/2014 12:50 PM, Cole Robinson wrote:
Remove the original VolOpen implementation, which is now only used in one spot. --- src/storage/storage_backend.c | 14 +++----------- src/storage/storage_backend.h | 8 ++------ src/storage/storage_backend_fs.c | 2 +- src/storage/storage_backend_logical.c | 4 +++- 4 files changed, 9 insertions(+), 19 deletions(-)
ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 03/31/2014 03:20 PM, Eric Blake wrote:
On 03/31/2014 12:50 PM, Cole Robinson wrote:
Remove the original VolOpen implementation, which is now only used in one spot. --- src/storage/storage_backend.c | 14 +++----------- src/storage/storage_backend.h | 8 ++------ src/storage/storage_backend_fs.c | 2 +- src/storage/storage_backend_logical.c | 4 +++- 4 files changed, 9 insertions(+), 19 deletions(-)
ACK
Thanks, I've pushed the first 3 patches. - Cole

VolOpen notifies the user of a potentially non-fatal failure by returning -2 and logging a VIR_WARN or VIR_INFO. Unfortunately most callers treat -2 as fatal but don't actually report any message with the error APIs. Change VolOpen to report an error if the VOL_OPEN_ERROR flag is passed. The ony caller that doesn't use this flag is the one that is explicitly handling -2 return code, so it fits the pattern. Tweak some of the other call sites to propagate the newly raised error. --- src/storage/storage_backend.c | 60 ++++++++++++++++++++++++----------- src/storage/storage_backend_fs.c | 21 ++++++------ src/storage/storage_backend_logical.c | 6 +++- src/storage/storage_backend_scsi.c | 4 +-- 4 files changed, 59 insertions(+), 32 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 42bd445..7c1220e 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1273,10 +1273,11 @@ virStorageBackendDetectBlockVolFormatFD(virStorageVolTargetPtr target, /* - * Allows caller to silently ignore files with improper mode - * * Returns -1 on error, -2 if file mode is unexpected or the * volume is a dangling symbolic link. + * + * -2 can be an ignorable error, but callers have to make sure to + * virResetLastError() */ int virStorageBackendVolOpen(const char *path, struct stat *sb, @@ -1284,22 +1285,30 @@ virStorageBackendVolOpen(const char *path, struct stat *sb, { int fd, mode = 0; char *base = last_component(path); + bool raise_error = (flags & VIR_STORAGE_VOL_OPEN_ERROR); if (lstat(path, sb) < 0) { - if (errno == ENOENT && !(flags & VIR_STORAGE_VOL_OPEN_ERROR)) { + if (errno == ENOENT && !raise_error) { VIR_WARN("ignoring missing file '%s'", path); return -2; } - virReportSystemError(errno, - _("cannot stat file '%s'"), - path); + + virReportSystemError(errno, _("cannot stat file '%s'"), path); return -1; } if (S_ISFIFO(sb->st_mode)) { + if (raise_error) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Volume path '%s' is a FIFO"), path); + } VIR_WARN("ignoring FIFO '%s'", path); return -2; } else if (S_ISSOCK(sb->st_mode)) { + if (raise_error) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Volume path '%s' is a socket"), path); + } VIR_WARN("ignoring socket '%s'", path); return -2; } @@ -1311,26 +1320,25 @@ virStorageBackendVolOpen(const char *path, struct stat *sb, * blocking fd, so fix it up after verifying we avoided a * race. */ if ((fd = open(path, O_RDONLY|O_NONBLOCK|O_NOCTTY)) < 0) { + if (raise_error) { + virReportSystemError(errno, _("cannot open volume '%s'"), path); + } + if ((errno == ENOENT || errno == ELOOP) && S_ISLNK(sb->st_mode)) { VIR_WARN("ignoring dangling symlink '%s'", path); return -2; } - if (errno == ENOENT && !(flags & VIR_STORAGE_VOL_OPEN_ERROR)) { + if (errno == ENOENT && !raise_error) { VIR_WARN("ignoring missing file '%s'", path); return -2; } - virReportSystemError(errno, - _("cannot open volume '%s'"), - path); return -1; } if (fstat(fd, sb) < 0) { - virReportSystemError(errno, - _("cannot stat file '%s'"), - path); + virReportSystemError(errno, _("cannot stat file '%s'"), path); VIR_FORCE_CLOSE(fd); return -1; } @@ -1347,18 +1355,30 @@ virStorageBackendVolOpen(const char *path, struct stat *sb, if (STREQ(base, ".") || STREQ(base, "..")) { VIR_FORCE_CLOSE(fd); + if (raise_error) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot use volume path '%s'"), path); + } VIR_INFO("Skipping special dir '%s'", base); return -2; } } else { + if (raise_error) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected type for file '%s'"), path); + } VIR_WARN("ignoring unexpected type for file '%s'", path); VIR_FORCE_CLOSE(fd); return -2; } if (virSetBlocking(fd, true) < 0) { - virReportSystemError(errno, _("unable to set blocking mode for '%s'"), - path); + if (raise_error) { + virReportSystemError(errno, + _("unable to set blocking mode for '%s'"), + path); + } + VIR_WARN("Unable to set blocking mode for '%s'", path); VIR_FORCE_CLOSE(fd); return -2; } @@ -1366,13 +1386,11 @@ virStorageBackendVolOpen(const char *path, struct stat *sb, if (!(mode & flags)) { VIR_FORCE_CLOSE(fd); VIR_INFO("Skipping volume '%s'", path); - - if (mode & VIR_STORAGE_VOL_OPEN_ERROR) { + if (raise_error) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected storage mode for '%s'"), path); return -1; } - return -2; } @@ -1389,8 +1407,12 @@ virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target, int ret, fd = -1; struct stat sb; - if ((ret = virStorageBackendVolOpen(target->path, &sb, openflags)) < 0) + if ((ret = virStorageBackendVolOpen(target->path, &sb, openflags)) < 0) { + /* ret == -2 may be a non-fatal error, but if callers want that + * behavior they should call VolOpen manually */ + ret = -1; goto cleanup; + } fd = ret; if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index e02d17f..4d44897 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -82,8 +82,11 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target, *encryption = NULL; if ((ret = virStorageBackendVolOpen(target->path, &sb, - VIR_STORAGE_VOL_FS_REFRESH_FLAGS)) < 0) - goto error; /* Take care to propagate ret, it is not always -1 */ + VIR_STORAGE_VOL_FS_REFRESH_FLAGS)) < 0) { + /* ret == -2 is non-fatal, propage the return code so the caller + * can handle it */ + goto error; + } fd = ret; if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb, @@ -904,8 +907,9 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, * have logged a similar message for the same problem, but only * if AUTO format detection was used. */ virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot probe backing volume info: %s"), - vol->backingStore.path); + _("cannot probe backing volume path '%s': %s"), + vol->backingStore.path, + virGetLastErrorMessage()); } } @@ -1177,13 +1181,10 @@ virStorageBackendFileSystemVolRefresh(virConnectPtr conn, virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr vol) { - int ret; - /* Refresh allocation / permissions info in case its changed */ - ret = virStorageBackendUpdateVolInfo(vol, false, false, - VIR_STORAGE_VOL_FS_OPEN_FLAGS); - if (ret < 0) - return ret; + if (virStorageBackendUpdateVolInfo(vol, false, false, + VIR_STORAGE_VOL_FS_OPEN_FLAGS) < 0) + return -1; /* Load any secrets if posible */ if (vol->target.encryption && diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 7893626..4699dfb 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -762,8 +762,12 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, cmd = NULL; if ((fd = virStorageBackendVolOpen(vol->target.path, &sb, - VIR_STORAGE_VOL_OPEN_DEFAULT)) < 0) + VIR_STORAGE_VOL_OPEN_DEFAULT)) < 0) { + /* This returns -2 for potentially skipable errors, but we + * want to report them all. */ + fd = -1; goto error; + } /* We can only chown/grp if root */ if (geteuid() == 0) { diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 4c2484d..51404ff 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -202,8 +202,8 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, if (virStorageBackendUpdateVolInfo(vol, true, true, VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to update volume for '%s'"), - devpath); + _("Failed to update volume for '%s': %s"), + devpath, virGetLastErrorMessage()); retval = -1; goto free_vol; } -- 1.8.5.3

On Mon, Mar 31, 2014 at 02:50:48PM -0400, Cole Robinson wrote:
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index e02d17f..4d44897 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -904,8 +907,9 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, * have logged a similar message for the same problem, but only * if AUTO format detection was used. */ virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot probe backing volume info: %s"), - vol->backingStore.path); + _("cannot probe backing volume path '%s': %s"), + vol->backingStore.path, + virGetLastErrorMessage());
[snip]
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 4c2484d..51404ff 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -202,8 +202,8 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, if (virStorageBackendUpdateVolInfo(vol, true, true, VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to update volume for '%s'"), - devpath); + _("Failed to update volume for '%s': %s"), + devpath, virGetLastErrorMessage()); retval = -1; goto free_vol;
The fact that you can call virGetLastErrorMessage() here is showing that we're overwriting earlier errors. IMHO use of virGetLastErrorMessage() is exclusivly for client applicatons / test suites. We should just not overwrite the original errors here. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 03/31/2014 03:08 PM, Daniel P. Berrange wrote:
On Mon, Mar 31, 2014 at 02:50:48PM -0400, Cole Robinson wrote:
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index e02d17f..4d44897 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -904,8 +907,9 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, * have logged a similar message for the same problem, but only * if AUTO format detection was used. */ virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot probe backing volume info: %s"), - vol->backingStore.path); + _("cannot probe backing volume path '%s': %s"), + vol->backingStore.path, + virGetLastErrorMessage());
[snip]
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 4c2484d..51404ff 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -202,8 +202,8 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, if (virStorageBackendUpdateVolInfo(vol, true, true, VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to update volume for '%s'"), - devpath); + _("Failed to update volume for '%s': %s"), + devpath, virGetLastErrorMessage()); retval = -1; goto free_vol;
The fact that you can call virGetLastErrorMessage() here is showing that we're overwriting earlier errors. IMHO use of virGetLastErrorMessage() is exclusivly for client applicatons / test suites. We should just not overwrite the original errors here.
Regards, Daniel
Indeed in this case it's not that useful, I was just preserving original behavior. I'll drop it in the next round. Thanks, Cole

On 03/31/2014 12:50 PM, Cole Robinson wrote:
VolOpen notifies the user of a potentially non-fatal failure by returning -2 and logging a VIR_WARN or VIR_INFO. Unfortunately most callers treat -2 as fatal but don't actually report any message with the error APIs.
Change VolOpen to report an error if the VOL_OPEN_ERROR flag is passed. The ony caller that doesn't use this flag is the one that is explicitly handling -2 return code, so it fits the pattern.
Tweak some of the other call sites to propagate the newly raised error. --- src/storage/storage_backend.c | 60 ++++++++++++++++++++++++----------- src/storage/storage_backend_fs.c | 21 ++++++------ src/storage/storage_backend_logical.c | 6 +++- src/storage/storage_backend_scsi.c | 4 +-- 4 files changed, 59 insertions(+), 32 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 42bd445..7c1220e 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1273,10 +1273,11 @@ virStorageBackendDetectBlockVolFormatFD(virStorageVolTargetPtr target,
/* - * Allows caller to silently ignore files with improper mode - * * Returns -1 on error, -2 if file mode is unexpected or the * volume is a dangling symbolic link. + * + * -2 can be an ignorable error, but callers have to make sure to + * virResetLastError() */
Is this comment still accurate?
char *base = last_component(path); + bool raise_error = (flags & VIR_STORAGE_VOL_OPEN_ERROR);
Hmm - are the semantics of this flag backwards? If most users always want an error, and only one specific caller wants the error suppressed because it is prepared to deal with a -2 return, shouldn't the default be errors when flags==0, and the one caller pass a non-zero flag for VIR_STORAGE_VOL_OPEN_NOERROR?
if (lstat(path, sb) < 0) { - if (errno == ENOENT && !(flags & VIR_STORAGE_VOL_OPEN_ERROR)) { + if (errno == ENOENT && !raise_error) { VIR_WARN("ignoring missing file '%s'", path); return -2; } - virReportSystemError(errno, - _("cannot stat file '%s'"), - path); + + virReportSystemError(errno, _("cannot stat file '%s'"), path); return -1;
This is an unconditional error, even when the flags say the user doesn't want errors; do we need to tweak the docs to mention that we are only suppressing SOME errors, not all?
@@ -82,8 +82,11 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target, *encryption = NULL;
if ((ret = virStorageBackendVolOpen(target->path, &sb, - VIR_STORAGE_VOL_FS_REFRESH_FLAGS)) < 0) - goto error; /* Take care to propagate ret, it is not always -1 */ + VIR_STORAGE_VOL_FS_REFRESH_FLAGS)) < 0) { + /* ret == -2 is non-fatal, propage the return code so the caller
s/propage/propagate/ -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 03/31/2014 08:50 PM, Cole Robinson wrote:
VolOpen notifies the user of a potentially non-fatal failure by returning -2 and logging a VIR_WARN or VIR_INFO. Unfortunately most callers treat -2 as fatal but don't actually report any message with the error APIs.
Change VolOpen to report an error if the VOL_OPEN_ERROR flag is passed. The ony caller that doesn't use this flag is the one that is explicitly handling -2 return code, so it fits the pattern.
Tweak some of the other call sites to propagate the newly raised error. --- src/storage/storage_backend.c | 60 ++++++++++++++++++++++++----------- src/storage/storage_backend_fs.c | 21 ++++++------ src/storage/storage_backend_logical.c | 6 +++- src/storage/storage_backend_scsi.c | 4 +-- 4 files changed, 59 insertions(+), 32 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 42bd445..7c1220e 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1273,10 +1273,11 @@ virStorageBackendDetectBlockVolFormatFD(virStorageVolTargetPtr target,
/* - * Allows caller to silently ignore files with improper mode - * * Returns -1 on error, -2 if file mode is unexpected or the * volume is a dangling symbolic link. + * + * -2 can be an ignorable error, but callers have to make sure to + * virResetLastError() */ int virStorageBackendVolOpen(const char *path, struct stat *sb, @@ -1284,22 +1285,30 @@ virStorageBackendVolOpen(const char *path, struct stat *sb, { int fd, mode = 0; char *base = last_component(path); + bool raise_error = (flags & VIR_STORAGE_VOL_OPEN_ERROR);
We've been using !!(flags & FLAG) for this kind of assignment, but I can't remember what that fixes.
if (lstat(path, sb) < 0) { - if (errno == ENOENT && !(flags & VIR_STORAGE_VOL_OPEN_ERROR)) { + if (errno == ENOENT && !raise_error) { VIR_WARN("ignoring missing file '%s'", path); return -2; } - virReportSystemError(errno, - _("cannot stat file '%s'"), - path); + + virReportSystemError(errno, _("cannot stat file '%s'"), path); return -1; }
if (S_ISFIFO(sb->st_mode)) { + if (raise_error) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Volume path '%s' is a FIFO"), path); + } VIR_WARN("ignoring FIFO '%s'", path);
We don't need both an error and a warning. And I think we should return -1 if an error has been logged.
return -2; } else if (S_ISSOCK(sb->st_mode)) { + if (raise_error) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Volume path '%s' is a socket"), path); + } VIR_WARN("ignoring socket '%s'", path); return -2; }
@@ -1366,13 +1386,11 @@ virStorageBackendVolOpen(const char *path, struct stat *sb, if (!(mode & flags)) { VIR_FORCE_CLOSE(fd); VIR_INFO("Skipping volume '%s'", path); - - if (mode & VIR_STORAGE_VOL_OPEN_ERROR) {
Hmm, it seems this condition could never have been true before.
+ if (raise_error) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected storage mode for '%s'"), path); return -1; } - return -2; }
@@ -1389,8 +1407,12 @@ virStorageBackendUpdateVolTargetInfo(virStorageVolTargetPtr target, int ret, fd = -1; struct stat sb;
- if ((ret = virStorageBackendVolOpen(target->path, &sb, openflags)) < 0) + if ((ret = virStorageBackendVolOpen(target->path, &sb, openflags)) < 0) { + /* ret == -2 may be a non-fatal error, but if callers want that + * behavior they should call VolOpen manually */ + ret = -1; goto cleanup;
This and the snipped changes below would not be needed if we never returned -2 with VOL_OPEN_ERROR (or by default if we switch the flag to NOERROR as Eric suggested.)
+ } fd = ret;
if ((ret = virStorageBackendUpdateVolTargetInfoFD(target,
Jan

On 04/01/2014 02:56 AM, Ján Tomko wrote:
On 03/31/2014 08:50 PM, Cole Robinson wrote:
VolOpen notifies the user of a potentially non-fatal failure by returning -2 and logging a VIR_WARN or VIR_INFO. Unfortunately most callers treat -2 as fatal but don't actually report any message with the error APIs.
+ bool raise_error = (flags & VIR_STORAGE_VOL_OPEN_ERROR);
We've been using !!(flags & FLAG) for this kind of assignment, but I can't remember what that fixes.
It fixes the fact that gnulib's stdbool module works around C89 compilers that lack a native _Bool type, and therefore might assign a value other than 1 into what is supposed to be 0 or 1. But since we use other C99 features, it probably doesn't hurt us to assume that assignments to bool are handled correctly by the compiler. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (4)
-
Cole Robinson
-
Daniel P. Berrange
-
Eric Blake
-
Ján Tomko