Re: [libvirt] [PATCH 4/6] qemu: driver: Improve error suppression in qemuDomainStorageUpdatePhysical

[Ján Tomko]

On Wed, Aug 14, 2019 at 06:59:19PM +0200, Peter Krempa wrote:

None of the callers of qemuDomainStorageUpdatePhysical care about
errors.

Use the new flag for qemuDomainStorageOpenStat which suppresses some
errors and move the reset of the rest of the uncommon errors into this
function. Document what is happening in a comment for the function.

Signed-off-by: Peter Krempa 
---
src/qemu/qemu_driver.c | 24 ++--
1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cc296c1fe3..2dffef9642 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12318,6 +12318,19 @@ qemuDomainStorageCloseStat(virStorageSourcePtr src,
}


+/**
+ * qemuDomainStorageUpdatePhysical:
+ * @driver: qemu driver
+ * @cfg: qemu driver configuration object
+ * @vm: domain object
+ * @src: storage source to update
+ *
+ * Update the physical size of the disk by reading the actual size of the image
+ * on disk.
+ *
+ * Returns 0 on successful update and -1 otherwise (some uncommon errors may be
+ * reported but are reset (thus only logged)).


By uncommon you mean missing network storage? :)


+ */
static int
qemuDomainStorageUpdatePhysical(virQEMUDriverPtr driver,
virQEMUDriverConfigPtr cfg,
@@ -12331,8 +12344,11 @@ qemuDomainStorageUpdatePhysical(virQEMUDriverPtr 
driver,
if (virStorageSourceIsEmpty(src))
return 0;

-if (qemuDomainStorageOpenStat(driver, cfg, vm, src, &fd, &sb, false) < 0)
+if ((ret = qemuDomainStorageOpenStat(driver, cfg, vm, src, &fd, &sb, true)) 
<= 0) {
+if (ret < 0)
+virResetLastError();


Still ugly to use virResetLastError, but at least it's an improvement.

Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 4/6] qemu: driver: Improve error suppression in qemuDomainStorageUpdatePhysical

[Peter Krempa]

None of the callers of qemuDomainStorageUpdatePhysical care about
errors.

Use the new flag for qemuDomainStorageOpenStat which suppresses some
errors and move the reset of the rest of the uncommon errors into this
function. Document what is happening in a comment for the function.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_driver.c | 24 ++--
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cc296c1fe3..2dffef9642 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12318,6 +12318,19 @@ qemuDomainStorageCloseStat(virStorageSourcePtr src,
 }


+/**
+ * qemuDomainStorageUpdatePhysical:
+ * @driver: qemu driver
+ * @cfg: qemu driver configuration object
+ * @vm: domain object
+ * @src: storage source to update
+ *
+ * Update the physical size of the disk by reading the actual size of the image
+ * on disk.
+ *
+ * Returns 0 on successful update and -1 otherwise (some uncommon errors may be
+ * reported but are reset (thus only logged)).
+ */
 static int
 qemuDomainStorageUpdatePhysical(virQEMUDriverPtr driver,
 virQEMUDriverConfigPtr cfg,
@@ -12331,8 +12344,11 @@ qemuDomainStorageUpdatePhysical(virQEMUDriverPtr 
driver,
 if (virStorageSourceIsEmpty(src))
 return 0;

-if (qemuDomainStorageOpenStat(driver, cfg, vm, src, &fd, &sb, false) < 0)
+if ((ret = qemuDomainStorageOpenStat(driver, cfg, vm, src, &fd, &sb, 
true)) <= 0) {
+if (ret < 0)
+virResetLastError();
 return -1;
+}

 ret = virStorageSourceUpdatePhysicalSize(src, fd, &sb);

@@ -12504,7 +12520,6 @@ qemuDomainGetBlockInfo(virDomainPtr dom,
 if (qemuDomainStorageUpdatePhysical(driver, cfg, vm, disk->src) == 0) {
 info->physical = disk->src->physical;
 } else {
-virResetLastError();
 info->physical = entry->physical;
 }
 } else {
@@ -21376,12 +21391,9 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver,
 QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx,
  "physical", entry->physical);
 } else {
-if (qemuDomainStorageUpdatePhysical(driver, cfg, dom, src) == 0) {
+if (qemuDomainStorageUpdatePhysical(driver, cfg, dom, src) == 0)
 QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx,
  "physical", src->physical);
-} else {
-virResetLastError();
-}
 }

 ret = 0;
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list