[libvirt PATCH 00/14] Various cleanups
Some reported by cppcheck, maybe even Coverity, the rest I noticed while looking at the code. Ján Tomko (14): tools: virshCheckpointListCollect: remove unused names vbox: remove VBoxCGlueTerm vbox: StartMachine: overwrite ret less often Remove pointless assignments Remove pointless initializations virsh: virshStreamSourceSkip: remove unused 'off' Do not check whether unsigned variables are negative libxl: remove unused 'bits' from struct guest_arch api: virDomainMemoryStats: use 'ret' variable storage: createFileDir: remove useless 'err' variable storage: createFileDir: use less ternary operators vbox: reduce variable scope in vboxDumpStorageControllers storage: storageBackendWipeLocal: reduce variable scope Reduce scope of some variables examples/c/admin/logging.c | 2 +- src/libvirt-domain.c | 8 ++--- src/libvirt-host.c | 4 +-- src/libxl/libxl_capabilities.c | 1 - src/libxl/libxl_driver.c | 2 +- src/node_device/node_device_driver.c | 3 +- src/openvz/openvz_driver.c | 2 +- src/storage/storage_backend_mpath.c | 3 +- src/storage/storage_util.c | 33 ++++++++++---------- src/test/test_driver.c | 2 +- src/vbox/vbox_XPCOMCGlue.c | 19 ------------ src/vbox/vbox_XPCOMCGlue.h | 1 - src/vbox/vbox_common.c | 46 +++++++++++++--------------- tests/virnumamock.c | 2 +- tests/virrandommock.c | 2 +- tools/virsh-checkpoint.c | 5 --- tools/virsh-pool.c | 3 +- tools/virsh-util.c | 3 +- 18 files changed, 53 insertions(+), 88 deletions(-) -- 2.26.2
Introduced by: commit 689beaa47c51fb49fafa992dd468116b8f6b0782 and unused since. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tools/virsh-checkpoint.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tools/virsh-checkpoint.c b/tools/virsh-checkpoint.c index 33e70515ad..f3c4fe90ba 100644 --- a/tools/virsh-checkpoint.c +++ b/tools/virsh-checkpoint.c @@ -581,7 +581,6 @@ virshCheckpointListCollect(vshControl *ctl, bool tree) { size_t i; - char **names = NULL; int count = -1; virDomainCheckpointPtr *chks; virshCheckpointListPtr checkpointlist = vshMalloc(ctl, @@ -628,10 +627,6 @@ virshCheckpointListCollect(vshControl *ctl, cleanup: virshCheckpointListFree(checkpointlist); - if (names && count > 0) - for (i = 0; i < count; i++) - VIR_FREE(names[i]); - VIR_FREE(names); return ret; } -- 2.26.2
On Wed, Sep 23, 2020 at 08:14:50PM +0200, Ján Tomko wrote:
Introduced by: commit 689beaa47c51fb49fafa992dd468116b8f6b0782 and unused since.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tools/virsh-checkpoint.c | 5 ----- 1 file changed, 5 deletions(-)
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
cppcheck reports: src/vbox/vbox_XPCOMCGlue.c:226:21: style: The statement 'if (hVBoxXPCOMC!=NULL) hVBoxXPCOMC=NULL' is logically equivalent to 'hVBoxXPCOMC=NULL'. [duplicateConditionalAssign] It does not matter anyway because this function is never called. Fixes: e1506cb4eb7eab96e7ded27a23f0d8ac9697ac2a Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_XPCOMCGlue.c | 19 ------------------- src/vbox/vbox_XPCOMCGlue.h | 1 - 2 files changed, 20 deletions(-) diff --git a/src/vbox/vbox_XPCOMCGlue.c b/src/vbox/vbox_XPCOMCGlue.c index 3cbb679110..2936ff0edb 100644 --- a/src/vbox/vbox_XPCOMCGlue.c +++ b/src/vbox/vbox_XPCOMCGlue.c @@ -217,25 +217,6 @@ VBoxCGlueInit(unsigned int *version) } -/** - * Terminate the C glue library. - */ -void -VBoxCGlueTerm(void) -{ - if (hVBoxXPCOMC != NULL) { -#if 0 /* VBoxRT.so doesn't like being reloaded. See @bugref{3725}. */ - dlclose(g_hVBoxXPCOMC); -#endif - hVBoxXPCOMC = NULL; - } - - pVBoxFuncs_v2_2 = NULL; - g_pfnGetFunctions = NULL; -} - - - /* * In XPCOM an array is represented by 1) a pointer to an array of pointers * that point to the items and 2) an unsigned int representing the number of diff --git a/src/vbox/vbox_XPCOMCGlue.h b/src/vbox/vbox_XPCOMCGlue.h index 517b02393f..d0e579482e 100644 --- a/src/vbox/vbox_XPCOMCGlue.h +++ b/src/vbox/vbox_XPCOMCGlue.h @@ -36,7 +36,6 @@ extern PFNVBOXGETXPCOMCFUNCTIONS g_pfnGetFunctions; int VBoxCGlueInit(unsigned int *version); -void VBoxCGlueTerm(void); typedef struct _vboxArray vboxArray; -- 2.26.2
On Wed, Sep 23, 2020 at 08:14:51PM +0200, Ján Tomko wrote:
cppcheck reports: src/vbox/vbox_XPCOMCGlue.c:226:21: style: The statement 'if (hVBoxXPCOMC!=NULL) hVBoxXPCOMC=NULL' is logically equivalent to 'hVBoxXPCOMC=NULL'. [duplicateConditionalAssign]
It does not matter anyway because this function is never called.
Fixes: e1506cb4eb7eab96e7ded27a23f0d8ac9697ac2a Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Use goto to jump over the ret = 0 assignment as is usual in rest of the code. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_common.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 9978741a64..99c7fbd117 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -2090,7 +2090,7 @@ vboxStartMachine(virDomainPtr dom, int maxDomID, IMachine *machine, vboxIID *iid int ret = -1; if (!data->vboxObj) - return ret; + return -1; VBOX_UTF8_TO_UTF16("FRONTEND/Type", &keyTypeUtf16); gVBoxAPI.UIMachine.GetExtraData(machine, keyTypeUtf16, &valueTypeUtf16); @@ -2177,7 +2177,7 @@ vboxStartMachine(virDomainPtr dom, int maxDomID, IMachine *machine, vboxIID *iid if (NS_FAILED(rc)) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("OpenRemoteSession/LaunchVMProcess failed, domain can't be started")); - ret = -1; + goto cleanup; } else { PRBool completed = 0; resultCodeUnion resultCode; @@ -2186,19 +2186,21 @@ vboxStartMachine(virDomainPtr dom, int maxDomID, IMachine *machine, vboxIID *iid rc = gVBoxAPI.UIProgress.GetCompleted(progress, &completed); if (NS_FAILED(rc)) { /* error */ - ret = -1; + goto cleanup; } gVBoxAPI.UIProgress.GetResultCode(progress, &resultCode); if (RC_FAILED(resultCode)) { /* error */ - ret = -1; + goto cleanup; } else { /* all ok set the domid */ dom->id = maxDomID + 1; - ret = 0; } } + ret = 0; + + cleanup: VBOX_RELEASE(progress); gVBoxAPI.UISession.Close(data->vboxSession); -- 2.26.2
On Wed, Sep 23, 2020 at 08:14:52PM +0200, Ján Tomko wrote:
Use goto to jump over the ret = 0 assignment as is usual in rest of the code.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_common.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 9978741a64..99c7fbd117 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -2090,7 +2090,7 @@ vboxStartMachine(virDomainPtr dom, int maxDomID, IMachine *machine, vboxIID *iid int ret = -1;
if (!data->vboxObj) - return ret; + return -1;
VBOX_UTF8_TO_UTF16("FRONTEND/Type", &keyTypeUtf16); gVBoxAPI.UIMachine.GetExtraData(machine, keyTypeUtf16, &valueTypeUtf16); @@ -2177,7 +2177,7 @@ vboxStartMachine(virDomainPtr dom, int maxDomID, IMachine *machine, vboxIID *iid if (NS_FAILED(rc)) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("OpenRemoteSession/LaunchVMProcess failed, domain can't be started")); - ret = -1; + goto cleanup; } else { PRBool completed = 0; resultCodeUnion resultCode; @@ -2186,19 +2186,21 @@ vboxStartMachine(virDomainPtr dom, int maxDomID, IMachine *machine, vboxIID *iid rc = gVBoxAPI.UIProgress.GetCompleted(progress, &completed); if (NS_FAILED(rc)) { /* error */ - ret = -1; + goto cleanup;
This one is not semantically equivalent. But since I do not know enough about the vbox driver I cannot tell whether it makes sense to keep the current behaviour. It looks like it also fixes it but I cannot be sure enough. Let's wait for a couple of days if someone knows more about that and if not, then consider this Reviewed-by: Martin Kletzander <mkletzan@redhat.com> and let's see if someone experiences issues after that.
} gVBoxAPI.UIProgress.GetResultCode(progress, &resultCode); if (RC_FAILED(resultCode)) { /* error */ - ret = -1; + goto cleanup; } else { /* all ok set the domid */ dom->id = maxDomID + 1; - ret = 0; } }
+ ret = 0; + + cleanup: VBOX_RELEASE(progress);
gVBoxAPI.UISession.Close(data->vboxSession); -- 2.26.2
There's no need to initialize every variable, especially not if it's overwritten two lines later. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- examples/c/admin/logging.c | 2 +- src/libxl/libxl_driver.c | 2 +- src/vbox/vbox_common.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/c/admin/logging.c b/examples/c/admin/logging.c index 730ae40d9d..c183c4867d 100644 --- a/examples/c/admin/logging.c +++ b/examples/c/admin/logging.c @@ -29,7 +29,7 @@ int main(int argc, char **argv) const char *set_outputs = NULL; const char *set_filters = NULL; - ret = c = -1; + ret = -1; opterr = 0; while ((c = getopt(argc, argv, ":hpo:f:")) > 0) { diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 161a6882f3..ea7d08cd11 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -5423,7 +5423,7 @@ libxlDiskSectorSize(int domid, int devno) return ret; } - path = val = NULL; + val = NULL; path = g_strdup_printf("/local/domain/%d/device/vbd/%d/backend", domid, devno); if ((val = xs_read(handle, XBT_NULL, path, &len)) == NULL) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 99c7fbd117..2783827012 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -4618,7 +4618,7 @@ vboxSnapshotRedefine(virDomainPtr dom, if (openSessionForMachine(data, dom->uuid, &domiid, &machine) < 0) goto cleanup; - rc = gVBoxAPI.UIMachine.SaveSettings(machine); + gVBoxAPI.UIMachine.SaveSettings(machine); /* It may failed when the machine is not mutable. */ rc = gVBoxAPI.UIMachine.GetSettingsFilePath(machine, &settingsFilePath); if (NS_FAILED(rc)) { -- 2.26.2
On Wed, Sep 23, 2020 at 08:14:53PM +0200, Ján Tomko wrote:
There's no need to initialize every variable,
Automotive and some other industries would wholeheartedly disagree with you, but that's not the point here. I would argue that initializing by default is a very good practice in larger codebases if only for error-proofing against assumptions in future changes. But what you are fixing are just assignments which are mostly pointless.
especially not if it's overwritten two lines later.
That's true. Or if it is unused (e.g. the last hunk).
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- examples/c/admin/logging.c | 2 +- src/libxl/libxl_driver.c | 2 +- src/vbox/vbox_common.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/examples/c/admin/logging.c b/examples/c/admin/logging.c index 730ae40d9d..c183c4867d 100644 --- a/examples/c/admin/logging.c +++ b/examples/c/admin/logging.c @@ -29,7 +29,7 @@ int main(int argc, char **argv) const char *set_outputs = NULL; const char *set_filters = NULL;
- ret = c = -1; + ret = -1; opterr = 0;
while ((c = getopt(argc, argv, ":hpo:f:")) > 0) { diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 161a6882f3..ea7d08cd11 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -5423,7 +5423,7 @@ libxlDiskSectorSize(int domid, int devno) return ret; }
- path = val = NULL; + val = NULL;
Why not get rid of the `val = NULL` as well?
path = g_strdup_printf("/local/domain/%d/device/vbd/%d/backend", domid, devno);
if ((val = xs_read(handle, XBT_NULL, path, &len)) == NULL)
It's rewritten right here.
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 99c7fbd117..2783827012 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -4618,7 +4618,7 @@ vboxSnapshotRedefine(virDomainPtr dom, if (openSessionForMachine(data, dom->uuid, &domiid, &machine) < 0) goto cleanup;
- rc = gVBoxAPI.UIMachine.SaveSettings(machine); + gVBoxAPI.UIMachine.SaveSettings(machine);
Maybe this is missing a check for an error instead. Impossible to find out from the docs that I quickly tried to look up though. Anyway, Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
/* It may failed when the machine is not mutable. */ rc = gVBoxAPI.UIMachine.GetSettingsFilePath(machine, &settingsFilePath); if (NS_FAILED(rc)) { -- 2.26.2
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/test/test_driver.c | 2 +- tests/virnumamock.c | 2 +- tests/virrandommock.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index d582c207f4..cbbfea6665 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4354,7 +4354,7 @@ testNodeGetFreePages(virConnectPtr conn G_GNUC_UNUSED, unsigned long long *counts, unsigned int flags) { - size_t i = 0, j = 0; + size_t i, j; int x = 6; virCheckFlags(0, -1); diff --git a/tests/virnumamock.c b/tests/virnumamock.c index 40e18e646e..d39c264a3f 100644 --- a/tests/virnumamock.c +++ b/tests/virnumamock.c @@ -130,7 +130,7 @@ virNumaGetPages(int node, { const int pages_def[] = { 4, 2 * 1024, 1 * 1024 * 1024}; const int npages_def = G_N_ELEMENTS(pages_def); - size_t i = 0; + size_t i; if (pages_size) *pages_size = g_new0(unsigned int, npages_def); diff --git a/tests/virrandommock.c b/tests/virrandommock.c index 6dd15213e3..ca0520a5a3 100644 --- a/tests/virrandommock.c +++ b/tests/virrandommock.c @@ -69,7 +69,7 @@ int gnutls_dh_params_generate2(gnutls_dh_params_t dparams, unsigned int bits) { - int rc = 0; + int rc; VIR_MOCK_REAL_INIT(gnutls_dh_params_generate2); -- 2.26.2
On Wed, Sep 23, 2020 at 08:14:54PM +0200, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/test/test_driver.c | 2 +- tests/virnumamock.c | 2 +- tests/virrandommock.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
As mentioned in the previous patch I do not think they are pointless in the long run. I prefer ret, rc, rv and similar to be initialised to error/negative value and others to their null values (numbers to zero, pointers to NULL etc.) even though I know this is a very subjective opinion. If you really want to push for this then it would require making Of course the compiler can check if the value is used before initialisation. And I hope all current compilers do that with our warning options. Except they cannot check for that if you only pass the address of the value. I guess these are fine, but I would not like to create a precedent. What I think we should do instead is make an exception for defining variables for iteration in the for loop itself. I know it's only 21 years since it was standardised and we want to make sure all the compilers and distros can handle it... /s OK, I understand that defining variables in the middle of the function most of the time clutters the code, but for 'i' and 'j' I, for one, see simply no reason to disallow that.
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index d582c207f4..cbbfea6665 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4354,7 +4354,7 @@ testNodeGetFreePages(virConnectPtr conn G_GNUC_UNUSED, unsigned long long *counts, unsigned int flags) { - size_t i = 0, j = 0; + size_t i, j; int x = 6;
virCheckFlags(0, -1); diff --git a/tests/virnumamock.c b/tests/virnumamock.c index 40e18e646e..d39c264a3f 100644 --- a/tests/virnumamock.c +++ b/tests/virnumamock.c @@ -130,7 +130,7 @@ virNumaGetPages(int node, { const int pages_def[] = { 4, 2 * 1024, 1 * 1024 * 1024}; const int npages_def = G_N_ELEMENTS(pages_def); - size_t i = 0; + size_t i;
if (pages_size) *pages_size = g_new0(unsigned int, npages_def); diff --git a/tests/virrandommock.c b/tests/virrandommock.c index 6dd15213e3..ca0520a5a3 100644 --- a/tests/virrandommock.c +++ b/tests/virrandommock.c @@ -69,7 +69,7 @@ int gnutls_dh_params_generate2(gnutls_dh_params_t dparams, unsigned int bits) { - int rc = 0; + int rc;
VIR_MOCK_REAL_INIT(gnutls_dh_params_generate2);
-- 2.26.2
On Thu, Sep 24, 2020 at 10:30:21AM +0200, Martin Kletzander wrote:
On Wed, Sep 23, 2020 at 08:14:54PM +0200, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/test/test_driver.c | 2 +- tests/virnumamock.c | 2 +- tests/virrandommock.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
As mentioned in the previous patch I do not think they are pointless in the long run. I prefer ret, rc, rv and similar to be initialised to error/negative value and others to their null values (numbers to zero, pointers to NULL etc.) even though I know this is a very subjective opinion.
Yeah, I tend to agree. From a defensive coding POV it is preferrable to initialize every single variable at time of declaration to a known value. While compilers and coverity can check for use of uninitialized variables, I don't think their checks have ever been perfect, because new versions of compilers tend to discover things periodically. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tools/virsh-util.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/virsh-util.c b/tools/virsh-util.c index b8b0f98066..af7ed55348 100644 --- a/tools/virsh-util.c +++ b/tools/virsh-util.c @@ -173,9 +173,8 @@ virshStreamSourceSkip(virStreamPtr st G_GNUC_UNUSED, { virshStreamCallbackDataPtr cbData = opaque; int fd = cbData->fd; - off_t cur; - if ((cur = lseek(fd, offset, SEEK_CUR)) == (off_t) -1) + if (lseek(fd, offset, SEEK_CUR) == (off_t) -1) return -1; return 0; -- 2.26.2
On Wed, Sep 23, 2020 at 08:14:55PM +0200, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tools/virsh-util.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
diff --git a/tools/virsh-util.c b/tools/virsh-util.c index b8b0f98066..af7ed55348 100644 --- a/tools/virsh-util.c +++ b/tools/virsh-util.c @@ -173,9 +173,8 @@ virshStreamSourceSkip(virStreamPtr st G_GNUC_UNUSED, { virshStreamCallbackDataPtr cbData = opaque; int fd = cbData->fd; - off_t cur;
- if ((cur = lseek(fd, offset, SEEK_CUR)) == (off_t) -1) + if (lseek(fd, offset, SEEK_CUR) == (off_t) -1) return -1;
return 0; -- 2.26.2
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/openvz/openvz_driver.c | 2 +- src/vbox/vbox_common.c | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 71e270ea09..74d87b1085 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1215,7 +1215,7 @@ static int openvzDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (!(vm = openvzDomObjFromDomain(driver, dom->uuid))) return -1; - if (nvcpus <= 0) { + if (nvcpus == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Number of vCPUs should be >= 1")); goto cleanup; diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 2783827012..b9b72e2e02 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -2111,7 +2111,7 @@ vboxStartMachine(virDomainPtr dom, int maxDomID, IMachine *machine, vboxIID *iid VBOX_UTF16_TO_UTF8(valueDisplayUtf16, &valueDisplayUtf8); VBOX_UTF16_FREE(valueDisplayUtf16); - if (strlen(valueDisplayUtf8) <= 0) + if (strlen(valueDisplayUtf8) == 0) VBOX_UTF8_FREE(valueDisplayUtf8); } @@ -2990,7 +2990,7 @@ vboxHostDeviceGetXMLDesc(vboxDriverPtr data, virDomainDefPtr def, IMachine *mach gVBoxAPI.UArray.vboxArrayGet(&deviceFilters, USBCommon, gVBoxAPI.UArray.handleUSBGetDeviceFilters(USBCommon)); - if (deviceFilters.count <= 0) + if (deviceFilters.count == 0) goto release_filters; /* check if the filters are active and then only @@ -3612,9 +3612,8 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine gVBoxAPI.UArray.vboxArrayGet(&sharedFolders, machine, gVBoxAPI.UArray.handleMachineGetSharedFolders(machine)); - if (sharedFolders.count <= 0) { - if (sharedFolders.count == 0) - ret = 0; + if (sharedFolders.count == 0) { + ret = 0; goto cleanup; } -- 2.26.2
On Wed, Sep 23, 2020 at 08:14:56PM +0200, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/openvz/openvz_driver.c | 2 +- src/vbox/vbox_common.c | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-)
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
It was made pointless by: commit c25c18f71bdc43a1305be4ad1a2ca91b25cf13f3 Convert capabilities / domain_conf to use virArch and unused by: commit 8db1f2d228bb2f27a729a873dcdb81ce3c7c38fd Fix libxl driver for virArch changes Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/libxl/libxl_capabilities.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c index 8284ea3d53..e5b144ea1d 100644 --- a/src/libxl/libxl_capabilities.c +++ b/src/libxl/libxl_capabilities.c @@ -52,7 +52,6 @@ enum libxlHwcapVersion { struct guest_arch { virArch arch; - int bits; int hvm; int pvh; int pae; -- 2.26.2
On Wed, Sep 23, 2020 at 08:14:57PM +0200, Ján Tomko wrote:
It was made pointless by: commit c25c18f71bdc43a1305be4ad1a2ca91b25cf13f3 Convert capabilities / domain_conf to use virArch
and unused by: commit 8db1f2d228bb2f27a729a873dcdb81ce3c7c38fd Fix libxl driver for virArch changes
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
--- src/libxl/libxl_capabilities.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c index 8284ea3d53..e5b144ea1d 100644 --- a/src/libxl/libxl_capabilities.c +++ b/src/libxl/libxl_capabilities.c @@ -52,7 +52,6 @@ enum libxlHwcapVersion {
struct guest_arch { virArch arch; - int bits; int hvm; int pvh; int pae; -- 2.26.2
Instead of 'nr_stats_ret'. Also reduce its scope. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/libvirt-domain.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index cde86c77e8..415482a526 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -5741,7 +5741,6 @@ virDomainMemoryStats(virDomainPtr dom, virDomainMemoryStatPtr stats, unsigned int nr_stats, unsigned int flags) { virConnectPtr conn; - unsigned long nr_stats_ret = 0; VIR_DOMAIN_DEBUG(dom, "stats=%p, nr_stats=%u, flags=0x%x", stats, nr_stats, flags); @@ -5758,11 +5757,10 @@ virDomainMemoryStats(virDomainPtr dom, virDomainMemoryStatPtr stats, conn = dom->conn; if (conn->driver->domainMemoryStats) { - nr_stats_ret = conn->driver->domainMemoryStats(dom, stats, nr_stats, - flags); - if (nr_stats_ret == -1) + int ret = conn->driver->domainMemoryStats(dom, stats, nr_stats, flags); + if (ret == -1) goto error; - return nr_stats_ret; + return ret; } virReportUnsupportedError(); -- 2.26.2
On Wed, Sep 23, 2020 at 08:14:58PM +0200, Ján Tomko wrote:
Instead of 'nr_stats_ret'. Also reduce its scope.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
--- src/libvirt-domain.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index cde86c77e8..415482a526 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -5741,7 +5741,6 @@ virDomainMemoryStats(virDomainPtr dom, virDomainMemoryStatPtr stats, unsigned int nr_stats, unsigned int flags) { virConnectPtr conn; - unsigned long nr_stats_ret = 0;
VIR_DOMAIN_DEBUG(dom, "stats=%p, nr_stats=%u, flags=0x%x", stats, nr_stats, flags); @@ -5758,11 +5757,10 @@ virDomainMemoryStats(virDomainPtr dom, virDomainMemoryStatPtr stats,
conn = dom->conn; if (conn->driver->domainMemoryStats) { - nr_stats_ret = conn->driver->domainMemoryStats(dom, stats, nr_stats, - flags); - if (nr_stats_ret == -1) + int ret = conn->driver->domainMemoryStats(dom, stats, nr_stats, flags); + if (ret == -1) goto error; - return nr_stats_ret; + return ret; }
virReportUnsupportedError(); -- 2.26.2
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/storage/storage_util.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 9171cb084f..93c24ab6bc 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -1997,7 +1997,6 @@ createFileDir(virStoragePoolObjPtr pool, unsigned int flags) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - int err; virCheckFlags(0, -1); @@ -2015,14 +2014,14 @@ createFileDir(virStoragePoolObjPtr pool, } - if ((err = virDirCreate(vol->target.path, - (vol->target.perms->mode == (mode_t)-1 ? - VIR_STORAGE_DEFAULT_VOL_PERM_MODE : - vol->target.perms->mode), - vol->target.perms->uid, - vol->target.perms->gid, - (def->type == VIR_STORAGE_POOL_NETFS - ? VIR_DIR_CREATE_AS_UID : 0))) < 0) { + if (virDirCreate(vol->target.path, + (vol->target.perms->mode == (mode_t)-1 ? + VIR_STORAGE_DEFAULT_VOL_PERM_MODE : + vol->target.perms->mode), + vol->target.perms->uid, + vol->target.perms->gid, + (def->type == VIR_STORAGE_POOL_NETFS + ? VIR_DIR_CREATE_AS_UID : 0)) < 0) { return -1; } -- 2.26.2
On Wed, Sep 23, 2020 at 08:14:59PM +0200, Ján Tomko wrote:
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
--- src/storage/storage_util.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 9171cb084f..93c24ab6bc 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -1997,7 +1997,6 @@ createFileDir(virStoragePoolObjPtr pool, unsigned int flags) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - int err;
virCheckFlags(0, -1);
@@ -2015,14 +2014,14 @@ createFileDir(virStoragePoolObjPtr pool, }
- if ((err = virDirCreate(vol->target.path, - (vol->target.perms->mode == (mode_t)-1 ? - VIR_STORAGE_DEFAULT_VOL_PERM_MODE : - vol->target.perms->mode), - vol->target.perms->uid, - vol->target.perms->gid, - (def->type == VIR_STORAGE_POOL_NETFS - ? VIR_DIR_CREATE_AS_UID : 0))) < 0) { + if (virDirCreate(vol->target.path, + (vol->target.perms->mode == (mode_t)-1 ? + VIR_STORAGE_DEFAULT_VOL_PERM_MODE : + vol->target.perms->mode), + vol->target.perms->uid, + vol->target.perms->gid, + (def->type == VIR_STORAGE_POOL_NETFS + ? VIR_DIR_CREATE_AS_UID : 0)) < 0) { return -1; }
-- 2.26.2
Introduce separate variables and if conditions with spaces around them to make the function call easier to read. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/storage/storage_util.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 93c24ab6bc..49ecbc5344 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -1997,6 +1997,8 @@ createFileDir(virStoragePoolObjPtr pool, unsigned int flags) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + mode_t permmode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE; + unsigned int flags = 0; virCheckFlags(0, -1); @@ -2013,15 +2015,17 @@ createFileDir(virStoragePoolObjPtr pool, return -1; } + if (vol->target.perms->mode != (mode_t)-1) + permmode = vol->target.perms->mode; + + if (def->type == VIR_STORAGE_POOL_NETFS) + flags |= VIR_DIR_CREATE_AS_UID; if (virDirCreate(vol->target.path, - (vol->target.perms->mode == (mode_t)-1 ? - VIR_STORAGE_DEFAULT_VOL_PERM_MODE : - vol->target.perms->mode), + permmode, vol->target.perms->uid, vol->target.perms->gid, - (def->type == VIR_STORAGE_POOL_NETFS - ? VIR_DIR_CREATE_AS_UID : 0)) < 0) { + flags) < 0) { return -1; } -- 2.26.2
On a Wednesday in 2020, Ján Tomko wrote:
Introduce separate variables and if conditions with spaces around them to make the function call easier to read.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/storage/storage_util.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 93c24ab6bc..49ecbc5344 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -1997,6 +1997,8 @@ createFileDir(virStoragePoolObjPtr pool, unsigned int flags)
This function already has flags ^
{ virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + mode_t permmode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE; + unsigned int flags = 0;
Consider s/flags/createflags/ squashed in from here on Jano
On Wed, Sep 23, 2020 at 08:15:00PM +0200, Ján Tomko wrote:
Introduce separate variables and if conditions with spaces around them to make the function call easier to read.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/storage/storage_util.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 93c24ab6bc..49ecbc5344 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -1997,6 +1997,8 @@ createFileDir(virStoragePoolObjPtr pool, unsigned int flags) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + mode_t permmode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE; + unsigned int flags = 0;
virCheckFlags(0, -1);
@@ -2013,15 +2015,17 @@ createFileDir(virStoragePoolObjPtr pool, return -1; }
+ if (vol->target.perms->mode != (mode_t)-1) + permmode = vol->target.perms->mode; +
It would be even less repetitive if you did: mode_t permmode = vol->target.perms->mode; if (permmode == -1) permmode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE; But Reviewed-by: Martin Kletzander <mkletzan@redhat.com> with your squash, of course.
+ if (def->type == VIR_STORAGE_POOL_NETFS) + flags |= VIR_DIR_CREATE_AS_UID;
if (virDirCreate(vol->target.path, - (vol->target.perms->mode == (mode_t)-1 ? - VIR_STORAGE_DEFAULT_VOL_PERM_MODE : - vol->target.perms->mode), + permmode, vol->target.perms->uid, vol->target.perms->gid, - (def->type == VIR_STORAGE_POOL_NETFS - ? VIR_DIR_CREATE_AS_UID : 0)) < 0) { + flags) < 0) { return -1; }
-- 2.26.2
Most of the variables were reinitialized on every iteration. Also remove the pointless initialization of 'i'. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_common.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index b9b72e2e02..6a517ff96c 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3079,23 +3079,18 @@ static int vboxDumpStorageControllers(virDomainDefPtr def, IMachine *machine) { vboxArray storageControllers = VBOX_ARRAY_INITIALIZER; - IStorageController *controller = NULL; - PRUint32 storageBus = StorageBus_Null; - PRUint32 controllerType = StorageControllerType_Null; - virDomainControllerDefPtr cont = NULL; - size_t i = 0; - int model = -1, ret = -1; - virDomainControllerType type = VIR_DOMAIN_CONTROLLER_TYPE_LAST; + int ret = -1; + size_t i; gVBoxAPI.UArray.vboxArrayGet(&storageControllers, machine, gVBoxAPI.UArray.handleMachineGetStorageControllers(machine)); for (i = 0; i < storageControllers.count; i++) { - controller = storageControllers.items[i]; - storageBus = StorageBus_Null; - controllerType = StorageControllerType_Null; - type = VIR_DOMAIN_CONTROLLER_TYPE_LAST; - model = -1; + IStorageController *controller = storageControllers.items[i]; + PRUint32 storageBus = StorageBus_Null; + PRUint32 controllerType = StorageControllerType_Null; + virDomainControllerType type = VIR_DOMAIN_CONTROLLER_TYPE_LAST; + int model = -1; if (!controller) continue; @@ -3133,8 +3128,6 @@ vboxDumpStorageControllers(virDomainDefPtr def, IMachine *machine) case StorageControllerType_IntelAhci: case StorageControllerType_I82078: case StorageControllerType_Null: - model = -1; - break; } @@ -3165,6 +3158,8 @@ vboxDumpStorageControllers(virDomainDefPtr def, IMachine *machine) } if (type != VIR_DOMAIN_CONTROLLER_TYPE_LAST) { + virDomainControllerDefPtr cont; + cont = virDomainDefAddController(def, type, -1, model); if (!cont) { virReportError(VIR_ERR_INTERNAL_ERROR, -- 2.26.2
On Wed, Sep 23, 2020 at 08:15:01PM +0200, Ján Tomko wrote:
Most of the variables were reinitialized on every iteration.
Also remove the pointless initialization of 'i'.
Same reply about the 'i' (👁), for the rest: Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_common.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index b9b72e2e02..6a517ff96c 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3079,23 +3079,18 @@ static int vboxDumpStorageControllers(virDomainDefPtr def, IMachine *machine) { vboxArray storageControllers = VBOX_ARRAY_INITIALIZER; - IStorageController *controller = NULL; - PRUint32 storageBus = StorageBus_Null; - PRUint32 controllerType = StorageControllerType_Null; - virDomainControllerDefPtr cont = NULL; - size_t i = 0; - int model = -1, ret = -1; - virDomainControllerType type = VIR_DOMAIN_CONTROLLER_TYPE_LAST; + int ret = -1; + size_t i;
gVBoxAPI.UArray.vboxArrayGet(&storageControllers, machine, gVBoxAPI.UArray.handleMachineGetStorageControllers(machine));
for (i = 0; i < storageControllers.count; i++) { - controller = storageControllers.items[i]; - storageBus = StorageBus_Null; - controllerType = StorageControllerType_Null; - type = VIR_DOMAIN_CONTROLLER_TYPE_LAST; - model = -1; + IStorageController *controller = storageControllers.items[i]; + PRUint32 storageBus = StorageBus_Null; + PRUint32 controllerType = StorageControllerType_Null; + virDomainControllerType type = VIR_DOMAIN_CONTROLLER_TYPE_LAST; + int model = -1;
if (!controller) continue; @@ -3133,8 +3128,6 @@ vboxDumpStorageControllers(virDomainDefPtr def, IMachine *machine) case StorageControllerType_IntelAhci: case StorageControllerType_I82078: case StorageControllerType_Null: - model = -1; - break; }
@@ -3165,6 +3158,8 @@ vboxDumpStorageControllers(virDomainDefPtr def, IMachine *machine) }
if (type != VIR_DOMAIN_CONTROLLER_TYPE_LAST) { + virDomainControllerDefPtr cont; + cont = virDomainDefAddController(def, type, -1, model); if (!cont) { virReportError(VIR_ERR_INTERNAL_ERROR, -- 2.26.2
Also use MIN instead of open-coding it. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/storage/storage_util.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 49ecbc5344..23632fc884 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -2526,10 +2526,8 @@ storageBackendWipeLocal(const char *path, size_t writebuf_length, bool zero_end) { - int written = 0; unsigned long long remaining = 0; off_t size; - size_t write_size = 0; g_autofree char *writebuf = NULL; if (VIR_ALLOC_N(writebuf, writebuf_length) < 0) @@ -2557,9 +2555,9 @@ storageBackendWipeLocal(const char *path, remaining = wipe_len; while (remaining > 0) { + size_t write_size = MIN(writebuf_length, remaining); + int written = safewrite(fd, writebuf, write_size); - write_size = (writebuf_length < remaining) ? writebuf_length : remaining; - written = safewrite(fd, writebuf, write_size); if (written < 0) { virReportSystemError(errno, _("Failed to write %zu bytes to " -- 2.26.2
On Wed, Sep 23, 2020 at 08:15:02PM +0200, Ján Tomko wrote:
Also use MIN instead of open-coding it.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
--- src/storage/storage_util.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 49ecbc5344..23632fc884 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -2526,10 +2526,8 @@ storageBackendWipeLocal(const char *path, size_t writebuf_length, bool zero_end) { - int written = 0; unsigned long long remaining = 0; off_t size; - size_t write_size = 0; g_autofree char *writebuf = NULL;
if (VIR_ALLOC_N(writebuf, writebuf_length) < 0) @@ -2557,9 +2555,9 @@ storageBackendWipeLocal(const char *path,
remaining = wipe_len; while (remaining > 0) { + size_t write_size = MIN(writebuf_length, remaining); + int written = safewrite(fd, writebuf, write_size);
- write_size = (writebuf_length < remaining) ? writebuf_length : remaining; - written = safewrite(fd, writebuf, write_size); if (written < 0) { virReportSystemError(errno, _("Failed to write %zu bytes to " -- 2.26.2
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/libvirt-host.c | 4 +--- src/node_device/node_device_driver.c | 3 ++- src/storage/storage_backend_mpath.c | 3 +-- src/storage/storage_util.c | 6 ++---- tools/virsh-pool.c | 3 ++- 5 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/libvirt-host.c b/src/libvirt-host.c index 07d13585f4..58622d415d 100644 --- a/src/libvirt-host.c +++ b/src/libvirt-host.c @@ -1352,8 +1352,6 @@ virConnectSetKeepAlive(virConnectPtr conn, int interval, unsigned int count) { - int ret = -1; - VIR_DEBUG("conn=%p, interval=%d, count=%u", conn, interval, count); virResetLastError(); @@ -1361,7 +1359,7 @@ virConnectSetKeepAlive(virConnectPtr conn, virCheckConnectReturn(conn, -1); if (conn->driver->connectSetKeepAlive) { - ret = conn->driver->connectSetKeepAlive(conn, interval, count); + int ret = conn->driver->connectSetKeepAlive(conn, interval, count); if (ret < 0) goto error; return ret; diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index f4b140bef4..f5ea973c7a 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -747,7 +747,6 @@ nodeDeviceCreateXML(virConnectPtr conn, g_autoptr(virNodeDeviceDef) def = NULL; g_autofree char *wwnn = NULL; g_autofree char *wwpn = NULL; - int parent_host = -1; virNodeDevicePtr device = NULL; const char *virt_type = NULL; @@ -765,6 +764,8 @@ nodeDeviceCreateXML(virConnectPtr conn, return NULL; if (nodeDeviceHasCapability(def, VIR_NODE_DEV_CAP_SCSI_HOST)) { + int parent_host; + if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) == -1) return NULL; diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index fffc0f86b7..f474ab32a9 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -151,13 +151,12 @@ static int virStorageBackendCreateVols(virStoragePoolObjPtr pool, struct dm_names *names) { - int is_mpath = 0; uint32_t minor = -1; uint32_t next; g_autofree char *map_device = NULL; do { - is_mpath = virStorageBackendIsMultipath(names->name); + int is_mpath = virStorageBackendIsMultipath(names->name); if (is_mpath < 0) return -1; diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 23632fc884..7bfcf53570 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -2416,7 +2416,6 @@ virStorageBackendVolUploadLocal(virStoragePoolObjPtr pool G_GNUC_UNUSED, unsigned int flags) { char *target_path = vol->target.path; - int has_snap = 0; bool sparse = flags & VIR_STORAGE_VOL_UPLOAD_SPARSE_STREAM; g_autofree char *path = NULL; @@ -2427,7 +2426,7 @@ virStorageBackendVolUploadLocal(virStoragePoolObjPtr pool G_GNUC_UNUSED, * when volUpload is fully finished. */ if (vol->target.format == VIR_STORAGE_FILE_PLOOP) { /* Fail if the volume contains snapshots or we failed to check it.*/ - has_snap = storageBackendPloopHasSnapshots(vol->target.path); + int has_snap = storageBackendPloopHasSnapshots(vol->target.path); if (has_snap < 0) { return -1; } else if (!has_snap) { @@ -2456,13 +2455,12 @@ virStorageBackendVolDownloadLocal(virStoragePoolObjPtr pool G_GNUC_UNUSED, unsigned int flags) { char *target_path = vol->target.path; - int has_snap = 0; bool sparse = flags & VIR_STORAGE_VOL_DOWNLOAD_SPARSE_STREAM; g_autofree char *path = NULL; virCheckFlags(VIR_STORAGE_VOL_DOWNLOAD_SPARSE_STREAM, -1); if (vol->target.format == VIR_STORAGE_FILE_PLOOP) { - has_snap = storageBackendPloopHasSnapshots(vol->target.path); + int has_snap = storageBackendPloopHasSnapshots(vol->target.path); if (has_snap < 0) { return -1; } else if (!has_snap) { diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 634e9ac8cd..497b409989 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -1583,7 +1583,6 @@ cmdPoolInfo(vshControl *ctl, const vshCmd *cmd) virStoragePoolInfo info; virStoragePoolPtr pool; int autostart = 0; - int persistent = 0; bool ret = true; bool bytes = false; char uuid[VIR_UUID_STRING_BUFLEN]; @@ -1601,6 +1600,8 @@ cmdPoolInfo(vshControl *ctl, const vshCmd *cmd) if (virStoragePoolGetInfo(pool, &info) == 0) { double val; const char *unit; + int persistent; + vshPrint(ctl, "%-15s %s\n", _("State:"), virshStoragePoolStateToString(info.state)); -- 2.26.2
participants (3)
-
Daniel P. Berrangé -
Ján Tomko -
Martin Kletzander