Re: [PATCH] virt-host-validate: Fix IOMMU output on aarch64
On Wed, Jun 9, 2021 at 11:22 AM Michal Prívozník wrote:
>
> On 6/8/21 10:16 PM, Fabiano Fidêncio wrote:
> > virt-host-validate should print "Checking for device assignment IOMMU
> > support" for all architectures, not only for Intel / AMD.
> >
> > This is the output without the patch:
> > ```
> > [fidencio@dentola libvirt]$ virt-host-validate
> > QEMU: comprobando if device /dev/kvm exists
> > : PASA
> > QEMU: comprobando if device /dev/kvm is accessible
> > : PASA
> > QEMU: comprobando if device /dev/vhost-net exists
> > : PASA
> > QEMU: comprobando if device /dev/net/tun exists
> > : PASA
> > QEMU: comprobando for cgroup 'cpu' controller support
> > : PASA
> > QEMU: comprobando for cgroup 'cpuacct' controller support
> > : PASA
> > QEMU: comprobando for cgroup 'cpuset' controller support
> > : PASA
> > QEMU: comprobando for cgroup 'memory' controller support
> > : PASA
> > QEMU: comprobando for cgroup 'devices' controller support
> > : ADVERTENCIA (Enable 'devices' in kernel Kconfig file or mount/enable
> > cgroup controller in your system)
> > QEMU: comprobando for cgroup 'blkio' controller support
> > : PASA
> > ADVERTENCIA (Unknown if this platform has IOMMU support)
> > QEMU: comprobando for secure guest support
> > : ADVERTENCIA (Unknown if this platform has Secure Guest support)
> >
> > ```
> >
> > This is the output with the patch:
> > ```
> > [fidencio@dentola libvirt]$ ./build/tools/virt-host-validate
> > QEMU: Checking if device /dev/kvm exists
> > : PASS
> > QEMU: Checking if device /dev/kvm is accessible
> > : PASS
> > QEMU: Checking if device /dev/vhost-net exists
> > : PASS
> > QEMU: Checking if device /dev/net/tun exists
> > : PASS
> > QEMU: Checking for cgroup 'cpu' controller support
> > : PASS
> > QEMU: Checking for cgroup 'cpuacct' controller support
> > : PASS
> > QEMU: Checking for cgroup 'cpuset' controller support
> > : PASS
> > QEMU: Checking for cgroup 'memory' controller support
> > : PASS
> > QEMU: Checking for cgroup 'devices' controller support
> > : WARN (Enable 'devices' in kernel Kconfig file or mount/enable cgroup
> > controller in your system)
> > QEMU: Checking for cgroup 'blkio' controller support
> > : PASS
> > QEMU: Checking for device assignment IOMMU support
> > : WARN (Unknown if this platform has IOMMU support)
> > QEMU: Checking for secure guest support
> > : WARN (Unknown if this platform has Secure Guest support)
> > ```
> >
> > Signed-off-by: Fabiano Fidêncio
> > ---
> > This is a follow-up on
> > https://listman.redhat.com/archives/libvir-list/2021-June/msg00190.html
> > ---
> > tools/virt-host-validate-common.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
>
> Almost good :-)
>
> > diff --git a/tools/virt-host-validate-common.c
> > b/tools/virt-host-validate-common.c
> > index 9412bb7514..ee22a88b31 100644
> > --- a/tools/virt-host-validate-common.c
> > +++ b/tools/virt-host-validate-common.c
> > @@ -335,6 +335,8 @@ int virHostValidateIOMMU(const char *hvname,
> > struct dirent *dent;
> > int rc;
> >
> > +virHostMsgCheck(hvname, "%s", _("for device assignment IOMMU
> > support"));
> > +
>
> So this prints out "checking for device ..." message (without \n at EOL)...
>
> > flags = virHostValidateGetCPUFlags();
> >
> > if (flags && virBitmapIsBitSet(flags, VIR_HOST_VALIDATE_CPU_FLAG_VMX))
> > @@ -345,7 +347,6 @@ int virHostValidateIOMMU(const char *hvname,
> > virBitmapFree(flags);
> >
> > if (isIntel) {
> > -virHostMsgCheck(hvname, "%s", _("for device assignment IOMMU
> > support"));
> > if (access("/sys/firmware/acpi/tables/DMAR", F_OK) == 0) {
> > virHostMsgPass()
Re: [PATCH 0/5] virth-host-validate: Couple of cleanups
On Tue, Jun 8, 2021 at 9:17 PM Fabiano Fidêncio wrote: > > On Tue, Jun 8, 2021 at 10:45 AM Michal Privoznik wrote: > > > > I've noticed couple of bugs/problems while reviewing Fabiano's patch. > > Here are fixes. > > > > Michal Prívozník (5): > > virt-host-validate: Initialize the error object > > virt-host-validate: Report an error if failed to detect CGroups > > virt-host-validate: Turn failure to read /proc/cmdline into an error > > virt-host-validate: Call VIR_HOST_VALIDATE_FAILURE() more frequently > > virHostValidateSecureGuests: Drop useless 'return 0' at the end > > > > tools/virt-host-validate-common.c | 28 +++- > > tools/virt-host-validate.c| 6 +- > > 2 files changed, 20 insertions(+), 14 deletions(-) > > > > -- > > 2.31.1 > > > Series: > > Reviewed-by: Fabiano Fidêncio > -- > Fabiano Fidêncio And while I have your attention, please, also consider: https://listman.redhat.com/archives/libvir-list/2021-June/msg00214.html I wrote that one as a follow-up to your series as it was just easier, sorry. Best Regards, -- Fabiano Fidêncio
[PATCH] virt-host-validate: Fix IOMMU output on aarch64
virt-host-validate should print "Checking for device assignment IOMMU
support" for all architectures, not only for Intel / AMD.
This is the output without the patch:
```
[fidencio@dentola libvirt]$ virt-host-validate
QEMU: comprobando if device /dev/kvm exists
: PASA
QEMU: comprobando if device /dev/kvm is accessible
: PASA
QEMU: comprobando if device /dev/vhost-net exists
: PASA
QEMU: comprobando if device /dev/net/tun exists
: PASA
QEMU: comprobando for cgroup 'cpu' controller support
: PASA
QEMU: comprobando for cgroup 'cpuacct' controller support
: PASA
QEMU: comprobando for cgroup 'cpuset' controller support
: PASA
QEMU: comprobando for cgroup 'memory' controller support
: PASA
QEMU: comprobando for cgroup 'devices' controller support
: ADVERTENCIA (Enable 'devices' in kernel Kconfig file or mount/enable cgroup
controller in your system)
QEMU: comprobando for cgroup 'blkio' controller support
: PASA
ADVERTENCIA (Unknown if this platform has IOMMU support)
QEMU: comprobando for secure guest support
: ADVERTENCIA (Unknown if this platform has Secure Guest support)
```
This is the output with the patch:
```
[fidencio@dentola libvirt]$ ./build/tools/virt-host-validate
QEMU: Checking if device /dev/kvm exists :
PASS
QEMU: Checking if device /dev/kvm is accessible:
PASS
QEMU: Checking if device /dev/vhost-net exists :
PASS
QEMU: Checking if device /dev/net/tun exists :
PASS
QEMU: Checking for cgroup 'cpu' controller support :
PASS
QEMU: Checking for cgroup 'cpuacct' controller support :
PASS
QEMU: Checking for cgroup 'cpuset' controller support :
PASS
QEMU: Checking for cgroup 'memory' controller support :
PASS
QEMU: Checking for cgroup 'devices' controller support :
WARN (Enable 'devices' in kernel Kconfig file or mount/enable cgroup controller
in your system)
QEMU: Checking for cgroup 'blkio' controller support :
PASS
QEMU: Checking for device assignment IOMMU support :
WARN (Unknown if this platform has IOMMU support)
QEMU: Checking for secure guest support:
WARN (Unknown if this platform has Secure Guest support)
```
Signed-off-by: Fabiano Fidêncio
---
This is a follow-up on
https://listman.redhat.com/archives/libvir-list/2021-June/msg00190.html
---
tools/virt-host-validate-common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virt-host-validate-common.c
b/tools/virt-host-validate-common.c
index 9412bb7514..ee22a88b31 100644
--- a/tools/virt-host-validate-common.c
+++ b/tools/virt-host-validate-common.c
@@ -335,6 +335,8 @@ int virHostValidateIOMMU(const char *hvname,
struct dirent *dent;
int rc;
+virHostMsgCheck(hvname, "%s", _("for device assignment IOMMU support"));
+
flags = virHostValidateGetCPUFlags();
if (flags && virBitmapIsBitSet(flags, VIR_HOST_VALIDATE_CPU_FLAG_VMX))
@@ -345,7 +347,6 @@ int virHostValidateIOMMU(const char *hvname,
virBitmapFree(flags);
if (isIntel) {
-virHostMsgCheck(hvname, "%s", _("for device assignment IOMMU
support"));
if (access("/sys/firmware/acpi/tables/DMAR", F_OK) == 0) {
virHostMsgPass();
bootarg = "intel_iommu=on";
@@ -357,7 +358,6 @@ int virHostValidateIOMMU(const char *hvname,
return VIR_HOST_VALIDATE_FAILURE(level);
}
} else if (isAMD) {
-virHostMsgCheck(hvname, "%s", _("for device assignment IOMMU
support"));
if (access("/sys/firmware/acpi/tables/IVRS", F_OK) == 0) {
virHostMsgPass();
bootarg = "iommu=pt iommu=1";
--
2.31.1
Re: [PATCH 0/5] virth-host-validate: Couple of cleanups
On Tue, Jun 8, 2021 at 10:45 AM Michal Privoznik wrote: > > I've noticed couple of bugs/problems while reviewing Fabiano's patch. > Here are fixes. > > Michal Prívozník (5): > virt-host-validate: Initialize the error object > virt-host-validate: Report an error if failed to detect CGroups > virt-host-validate: Turn failure to read /proc/cmdline into an error > virt-host-validate: Call VIR_HOST_VALIDATE_FAILURE() more frequently > virHostValidateSecureGuests: Drop useless 'return 0' at the end > > tools/virt-host-validate-common.c | 28 +++- > tools/virt-host-validate.c| 6 +- > 2 files changed, 20 insertions(+), 14 deletions(-) > > -- > 2.31.1 > Series: Reviewed-by: Fabiano Fidêncio -- Fabiano Fidêncio
Re: [PATCH 4/5] virt-host-validate: Call VIR_HOST_VALIDATE_FAILURE() more frequently
[...]
>
> if (virFileExists("/dev/sev")) {
> @@ -513,6 +515,7 @@ int virHostValidateSecureGuests(const char *hvname,
> virHostMsgFail(level,
> "AMD Secure Encrypted Virtualization appears to
> be "
> "disabled in firemare.");
Not related to this series, but: firemare -> firmware.
> +return VIR_HOST_VALIDATE_FAILURE(level);
> }
> } else {
> virHostMsgFail(level,
> --
> 2.31.1
>
Best Regards,
--
Fabiano Fidêncio
Re: [PATCH v2] tools: only fail validations if VIR_HOST_VALIDATE_FAIL is set
On Tue, Jun 8, 2021 at 8:51 AM Michal Prívozník wrote:
>
> On 6/7/21 6:22 PM, Fabiano Fidêncio wrote:
> > Currently `virt-host-validate` will fail whenever one of its calls fail,
> > regardless of virHostValidateLevel set.
> >
> > This behaviour is not optimal and makes it not exactly reliable as a
> > command line tool as other tools or scripts using it would have to check
> > its output to figure out whether something really failed or if a warning
> > was mistakenly treated as failure.
> >
> > With this change, the behaviour of whether to fail or not, is defined by
> > the caller of those functions, based on the virHostValidateLevel passed
> > to them.
> >
> > https://gitlab.com/libvirt/libvirt/-/issues/175
> >
> > Signed-off-by: Fabiano Fidêncio
> > ---
> > Changes since v1:
> > * Replace the `goto out;` and the `out` labels by the
> > `VIR_HOST_VALIDATE_FAILURE` macro
>
> Yeah, this opened pandora's box...
Oh yeah, it looked pretty much like that when I first looked at this
code over the weekend.
>
> > ---
> > tools/virt-host-validate-common.c | 30 +++---
> > tools/virt-host-validate-common.h | 14 ++
> > 2 files changed, 29 insertions(+), 15 deletions(-)
> >
> > diff --git a/tools/virt-host-validate-common.c
> > b/tools/virt-host-validate-common.c
> > index 6dd851f07d..9412bb7514 100644
> > --- a/tools/virt-host-validate-common.c
> > +++ b/tools/virt-host-validate-common.c
> > @@ -142,7 +142,7 @@ int virHostValidateDeviceExists(const char *hvname,
> >
> > if (access(dev_name, F_OK) < 0) {
> > virHostMsgFail(level, "%s", hint);
> > -return -1;
> > +return VIR_HOST_VALIDATE_FAILURE(level);
> > }
> >
> > virHostMsgPass();
> > @@ -159,7 +159,7 @@ int virHostValidateDeviceAccessible(const char *hvname,
> >
> > if (access(dev_name, R_OK|W_OK) < 0) {
> > virHostMsgFail(level, "%s", hint);
> > -return -1;
> > +return VIR_HOST_VALIDATE_FAILURE(level);
> > }
> >
> > virHostMsgPass();
> > @@ -180,7 +180,7 @@ int virHostValidateNamespace(const char *hvname,
> >
> > if (access(nspath, F_OK) < 0) {
> > virHostMsgFail(level, "%s", hint);
> > -return -1;
> > +return VIR_HOST_VALIDATE_FAILURE(level);
> > }
> >
> > virHostMsgPass();
> > @@ -264,17 +264,17 @@ int virHostValidateLinuxKernel(const char *hvname,
> >
> > if (STRNEQ(uts.sysname, "Linux")) {
> > virHostMsgFail(level, "%s", hint);
> > -return -1;
> > +return VIR_HOST_VALIDATE_FAILURE(level);
> > }
> >
> > if (virParseVersionString(uts.release, , true) < 0) {
> > virHostMsgFail(level, "%s", hint);
> > -return -1;
> > +return VIR_HOST_VALIDATE_FAILURE(level);
> > }
> >
> > if (thisversion < version) {
> > virHostMsgFail(level, "%s", hint);
> > -return -1;
> > +return VIR_HOST_VALIDATE_FAILURE(level);
>
> Up until here the use of VIR_HOST_VALIDATE_FAILURE() is good.
>
> > } else {
> > virHostMsgPass();
> > return 0;
> > @@ -291,7 +291,7 @@ int virHostValidateCGroupControllers(const char *hvname,
> > size_t i;
> >
> > if (virCgroupNew("/", -1, ) < 0)
> > -return -1;
> > +return VIR_HOST_VALIDATE_FAILURE(level);
>
> But this looks somewhat suspicious. What virCgroupNew() does is it
> detects controllers and their mountpoints (and return this in @group).
> Upon error (e.g. unable to open /proc/mounts or /proc/self/cgroups or
> failure in their parsing) a libvirt error is reported. And this reminds
> me of pandora's box. Firstly, the error object is never initialized
> (i.e. there's no virInitialize() call from main(), or better said before
> the first function that has potential of reporting an error). This leads
> to virResetError() (called from virReportError()) to free() random
> pointers.
>
> Secondly, I'm unsure what to do in this case. I mean, caller told us
> severity of this check (@level), so if we failed to initialize an
> internal structure we should honour caller's wish and do/do not return
> an error. BUT, with the way this code is currently written this function
> exits early, not printing anything anywhere. E.g.:
>
> QEMU: Checking if device /dev/net/tun exists
[PATCH v2] tools: only fail validations if VIR_HOST_VALIDATE_FAIL is set
Currently `virt-host-validate` will fail whenever one of its calls fail,
regardless of virHostValidateLevel set.
This behaviour is not optimal and makes it not exactly reliable as a
command line tool as other tools or scripts using it would have to check
its output to figure out whether something really failed or if a warning
was mistakenly treated as failure.
With this change, the behaviour of whether to fail or not, is defined by
the caller of those functions, based on the virHostValidateLevel passed
to them.
https://gitlab.com/libvirt/libvirt/-/issues/175
Signed-off-by: Fabiano Fidêncio
---
Changes since v1:
* Replace the `goto out;` and the `out` labels by the
`VIR_HOST_VALIDATE_FAILURE` macro
---
tools/virt-host-validate-common.c | 30 +++---
tools/virt-host-validate-common.h | 14 ++
2 files changed, 29 insertions(+), 15 deletions(-)
diff --git a/tools/virt-host-validate-common.c
b/tools/virt-host-validate-common.c
index 6dd851f07d..9412bb7514 100644
--- a/tools/virt-host-validate-common.c
+++ b/tools/virt-host-validate-common.c
@@ -142,7 +142,7 @@ int virHostValidateDeviceExists(const char *hvname,
if (access(dev_name, F_OK) < 0) {
virHostMsgFail(level, "%s", hint);
-return -1;
+return VIR_HOST_VALIDATE_FAILURE(level);
}
virHostMsgPass();
@@ -159,7 +159,7 @@ int virHostValidateDeviceAccessible(const char *hvname,
if (access(dev_name, R_OK|W_OK) < 0) {
virHostMsgFail(level, "%s", hint);
-return -1;
+return VIR_HOST_VALIDATE_FAILURE(level);
}
virHostMsgPass();
@@ -180,7 +180,7 @@ int virHostValidateNamespace(const char *hvname,
if (access(nspath, F_OK) < 0) {
virHostMsgFail(level, "%s", hint);
-return -1;
+return VIR_HOST_VALIDATE_FAILURE(level);
}
virHostMsgPass();
@@ -264,17 +264,17 @@ int virHostValidateLinuxKernel(const char *hvname,
if (STRNEQ(uts.sysname, "Linux")) {
virHostMsgFail(level, "%s", hint);
-return -1;
+return VIR_HOST_VALIDATE_FAILURE(level);
}
if (virParseVersionString(uts.release, , true) < 0) {
virHostMsgFail(level, "%s", hint);
-return -1;
+return VIR_HOST_VALIDATE_FAILURE(level);
}
if (thisversion < version) {
virHostMsgFail(level, "%s", hint);
-return -1;
+return VIR_HOST_VALIDATE_FAILURE(level);
} else {
virHostMsgPass();
return 0;
@@ -291,7 +291,7 @@ int virHostValidateCGroupControllers(const char *hvname,
size_t i;
if (virCgroupNew("/", -1, ) < 0)
-return -1;
+return VIR_HOST_VALIDATE_FAILURE(level);
for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
int flag = 1 << i;
@@ -303,7 +303,7 @@ int virHostValidateCGroupControllers(const char *hvname,
virHostMsgCheck(hvname, "for cgroup '%s' controller support", cg_name);
if (!virCgroupHasController(group, i)) {
-ret = -1;
+ret = VIR_HOST_VALIDATE_FAILURE(level);
virHostMsgFail(level, "Enable '%s' in kernel Kconfig file or "
"mount/enable cgroup controller in your system",
cg_name);
@@ -320,7 +320,7 @@ int virHostValidateCGroupControllers(const char *hvname
G_GNUC_UNUSED,
virHostValidateLevel level)
{
virHostMsgFail(level, "%s", "This platform does not support cgroups");
-return -1;
+return VIR_HOST_VALIDATE_FAILURE(level);
}
#endif /* !__linux__ */
@@ -354,7 +354,7 @@ int virHostValidateIOMMU(const char *hvname,
"No ACPI DMAR table found, IOMMU either "
"disabled in BIOS or not supported by this "
"hardware platform");
-return -1;
+return VIR_HOST_VALIDATE_FAILURE(level);
}
} else if (isAMD) {
virHostMsgCheck(hvname, "%s", _("for device assignment IOMMU
support"));
@@ -366,7 +366,7 @@ int virHostValidateIOMMU(const char *hvname,
"No ACPI IVRS table found, IOMMU either "
"disabled in BIOS or not supported by this "
"hardware platform");
-return -1;
+return VIR_HOST_VALIDATE_FAILURE(level);
}
} else if (ARCH_IS_PPC64(arch)) {
/* Empty Block */
@@ -385,7 +385,7 @@ int virHostValidateIOMMU(const char *hvname,
} else {
virHostMsgFail(level,
"Unknown if this platform has IOMMU support");
-return -1;
+return VIR_HOST_VALIDATE_FAILURE(level);
}
@@ -404,7
Re: [PATCH] tools: only fail validations if VIR_HOST_VALIDATE_FAIL is set
On Mon, Jun 7, 2021 at 10:45 AM Michal Prívozník wrote:
>
> On 6/6/21 12:15 PM, Fabiano Fidêncio wrote:
> > Currently `virt-host-validate` will fail whenever one of its calls fail,
> > regardless of virHostValidateLevel set.
> >
> > This behaviour is not optimal and makes it not exactly reliable as a
> > command line tool as other tools or scripts using it would have to check
> > its output to figure out whether something really failed or if a warning
> > was mistakenly treated as failure.
> >
> > With this change, the behaviour of whether to fail or not, is defined by
> > the caller of those functions, based on the virHostValidateLevel passed
> > to them.
> >
> > Signed-off-by: Fabiano Fidêncio
> > ---
> > tools/virt-host-validate-common.c | 129 ++
> > 1 file changed, 94 insertions(+), 35 deletions(-)
> >
> > diff --git a/tools/virt-host-validate-common.c
> > b/tools/virt-host-validate-common.c
> > index 6dd851f07d..2bf97bad75 100644
> > --- a/tools/virt-host-validate-common.c
> > +++ b/tools/virt-host-validate-common.c
> > @@ -138,15 +138,21 @@ int virHostValidateDeviceExists(const char *hvname,
> > virHostValidateLevel level,
> > const char *hint)
> > {
> > +int ret = 0;
> > +
> > virHostMsgCheck(hvname, "if device %s exists", dev_name);
> >
> > if (access(dev_name, F_OK) < 0) {
> > virHostMsgFail(level, "%s", hint);
> > -return -1;
> > +if (level == VIR_HOST_VALIDATE_FAIL)
> > +ret = -1;
> > +goto out;
> > }
> >
> > virHostMsgPass();
> > -return 0;
> > +
> > + out:
> > +return ret;
> > }
> >
> >
>
> The patch, or idea it implements is correct. However, I think a lot of
> these 'out' labels can be dropped and 'goto out' can be replaced with
> 'return -1' or 'return 0'. Does that work for you?
Sure, I'll submit a v2 later Today addressing those.
Thanks a whole lot for the timely review!
Best Regards,
--
Fabiano Fidêncio
Re: [PATCH] tools: only fail validations if VIR_HOST_VALIDATE_FAIL is set
[...]
> @@ -464,12 +514,15 @@ int virHostValidateSecureGuests(const char *hvname,
> if (!virFileIsDir("/sys/firmware/uv")) {
> virHostMsgFail(level, "IBM Secure Execution not supported by
> "
>"the currently used kernel");
> -return 0;
> +goto out;
> }
>
> -if (virFileReadValueString(, "/proc/cmdline") < 0)
> -return -1;
> -
> +if (virFileReadValueString(, "/proc/cmdline") < 0) {
> +if (level == VIR_HOST_VALIDATE_FAIL) {
> +ret = -1;
> +goto out;
Oops, this `goto out;` should be out of the if scope, sorry.
I'll fix this and re-submit a v2 after I get some reviews on this version.
[...]
Best Regards,
--
Fabiano Fidêncio
[PATCH] tools: only fail validations if VIR_HOST_VALIDATE_FAIL is set
Currently `virt-host-validate` will fail whenever one of its calls fail,
regardless of virHostValidateLevel set.
This behaviour is not optimal and makes it not exactly reliable as a
command line tool as other tools or scripts using it would have to check
its output to figure out whether something really failed or if a warning
was mistakenly treated as failure.
With this change, the behaviour of whether to fail or not, is defined by
the caller of those functions, based on the virHostValidateLevel passed
to them.
Signed-off-by: Fabiano Fidêncio
---
tools/virt-host-validate-common.c | 129 ++
1 file changed, 94 insertions(+), 35 deletions(-)
diff --git a/tools/virt-host-validate-common.c
b/tools/virt-host-validate-common.c
index 6dd851f07d..2bf97bad75 100644
--- a/tools/virt-host-validate-common.c
+++ b/tools/virt-host-validate-common.c
@@ -138,15 +138,21 @@ int virHostValidateDeviceExists(const char *hvname,
virHostValidateLevel level,
const char *hint)
{
+int ret = 0;
+
virHostMsgCheck(hvname, "if device %s exists", dev_name);
if (access(dev_name, F_OK) < 0) {
virHostMsgFail(level, "%s", hint);
-return -1;
+if (level == VIR_HOST_VALIDATE_FAIL)
+ret = -1;
+goto out;
}
virHostMsgPass();
-return 0;
+
+ out:
+return ret;
}
@@ -155,15 +161,21 @@ int virHostValidateDeviceAccessible(const char *hvname,
virHostValidateLevel level,
const char *hint)
{
+int ret = 0;
+
virHostMsgCheck(hvname, "if device %s is accessible", dev_name);
if (access(dev_name, R_OK|W_OK) < 0) {
virHostMsgFail(level, "%s", hint);
-return -1;
+if (level == VIR_HOST_VALIDATE_FAIL)
+ret = -1;
+goto out;
}
virHostMsgPass();
-return 0;
+
+ out:
+return ret;
}
@@ -173,6 +185,7 @@ int virHostValidateNamespace(const char *hvname,
const char *hint)
{
char nspath[100];
+int ret = 0;
virHostMsgCheck(hvname, "for namespace %s", ns_name);
@@ -180,11 +193,15 @@ int virHostValidateNamespace(const char *hvname,
if (access(nspath, F_OK) < 0) {
virHostMsgFail(level, "%s", hint);
-return -1;
+if (level == VIR_HOST_VALIDATE_FAIL)
+ret = -1;
+goto out;
}
virHostMsgPass();
-return 0;
+
+ out:
+return ret;
}
@@ -254,6 +271,7 @@ int virHostValidateLinuxKernel(const char *hvname,
{
struct utsname uts;
unsigned long thisversion;
+int ret = 0;
uname();
@@ -264,21 +282,29 @@ int virHostValidateLinuxKernel(const char *hvname,
if (STRNEQ(uts.sysname, "Linux")) {
virHostMsgFail(level, "%s", hint);
-return -1;
+if (level == VIR_HOST_VALIDATE_FAIL)
+ret = -1;
+goto out;
}
if (virParseVersionString(uts.release, , true) < 0) {
virHostMsgFail(level, "%s", hint);
-return -1;
+if (level == VIR_HOST_VALIDATE_FAIL)
+ret = -1;
+goto out;
}
if (thisversion < version) {
virHostMsgFail(level, "%s", hint);
-return -1;
-} else {
-virHostMsgPass();
-return 0;
+if (level == VIR_HOST_VALIDATE_FAIL)
+ret = -1;
+goto out;
}
+
+virHostMsgPass();
+
+ out:
+return ret;
}
#ifdef __linux__
@@ -290,8 +316,11 @@ int virHostValidateCGroupControllers(const char *hvname,
int ret = 0;
size_t i;
-if (virCgroupNew("/", -1, ) < 0)
-return -1;
+if (virCgroupNew("/", -1, ) < 0) {
+if (level == VIR_HOST_VALIDATE_FAIL)
+ret = -1;
+goto out;
+}
for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
int flag = 1 << i;
@@ -303,7 +332,8 @@ int virHostValidateCGroupControllers(const char *hvname,
virHostMsgCheck(hvname, "for cgroup '%s' controller support", cg_name);
if (!virCgroupHasController(group, i)) {
-ret = -1;
+if (level == VIR_HOST_VALIDATE_FAIL)
+ret = -1;
virHostMsgFail(level, "Enable '%s' in kernel Kconfig file or "
"mount/enable cgroup controller in your system",
cg_name);
@@ -312,6 +342,7 @@ int virHostValidateCGroupControllers(const char *hvname,
}
}
+ out:
return ret;
}
#else /* !__linux__ */
@@ -319,8 +350,13 @@ int virHostValidateCGroupControllers(const char *hvname
G_GNUC_UNUSED,
int controllers G_GNUC_UNUSED,
virHostValidateLevel l
Re: [PATCH 1/2] gobject: Fix typo in documentation of gvir_domain_save_to_file_async
On Wed, Mar 25, 2020 at 4:51 PM Felipe Borges wrote: > > Signed-off-by: Felipe Borges > --- > libvirt-gobject/libvirt-gobject-domain.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libvirt-gobject/libvirt-gobject-domain.c > b/libvirt-gobject/libvirt-gobject-domain.c > index 399b118..0a16cb4 100644 > --- a/libvirt-gobject/libvirt-gobject-domain.c > +++ b/libvirt-gobject/libvirt-gobject-domain.c > @@ -777,7 +777,7 @@ gvir_domain_save_to_file_helper(GTask *task, > * @filename: path to output file > * @custom_conf: (allow-none): configuration for domain or NULL > * @flags: the flags > - * @cancellable: (allow-none) (transfer none): cancallation object > + * @cancellable: (allow-none) (transfer none): cancellation object > * @callback: (scope async): completion callback > * @user_data: (closure): opaque data for callback > * > -- > 2.24.1 > > For both patches ... Reviewed-by: Fabiano Fidêncio ... and pushed. Do you need a release including 2nd patch in order to unblock something for Boxes on Fedora 32? Best Regards, -- Fabiano Fidêncio
Re: [libvirt-jenkins-ci PATCH 0/4] Fixes and improvements to cross-build environments
On Tue, Feb 25, 2020 at 1:17 PM Andrea Bolognani wrote: > > The first two commits are necessary to build container images that > can successfully be used in libosinfo's GitLab CI setup, which is > currently not performing MinGW builds because of the issues they > address. > > Andrea Bolognani (4): > lcitool: Install dpkg-dev when doing cross-builds on Debian > Don't set $PKG_CONFIG_LIBDIR anywhere > lcitool: Drop duplicated code > lcitool: Don't specify --target in $CONFIGURE_OPTS > > guests/lcitool | 15 +++ > guests/playbooks/build/jobs/defaults.yml | 2 -- > jenkins/jobs/defaults.yaml | 2 -- > 3 files changed, 3 insertions(+), 16 deletions(-) > > -- > 2.24.1 > Reviewed-by: Fabiano Fidêncio
[libvirt] [jenkins-ci PATCH v3 3/3] guests: Add projects to openSUSE 15.1
libvirt-tck and libvirt-cim could not be added due to missing packages.
Signed-off-by: Fabiano Fidêncio
---
guests/host_vars/libvirt-opensuse-151/main.yml | 14 ++
guests/playbooks/build/projects/libvirt-dbus.yml | 1 +
.../playbooks/build/projects/libvirt-sandbox.yml | 1 +
guests/playbooks/build/projects/virt-manager.yml | 2 ++
4 files changed, 18 insertions(+)
diff --git a/guests/host_vars/libvirt-opensuse-151/main.yml
b/guests/host_vars/libvirt-opensuse-151/main.yml
index f422a9e..88d5dfd 100644
--- a/guests/host_vars/libvirt-opensuse-151/main.yml
+++ b/guests/host_vars/libvirt-opensuse-151/main.yml
@@ -1,6 +1,20 @@
---
projects:
+ - gtk-vnc
+ - libosinfo
- libvirt
+ - libvirt-dbus
+ - libvirt-glib
+ - libvirt-go
+ - libvirt-go-xml
+ - libvirt-ocaml
+ - libvirt-perl
+ - libvirt-python
+ - libvirt-sandbox
+ - osinfo-db
+ - osinfo-db-tools
+ - virt-manager
+ - virt-viewer
package_format: 'rpm'
package_manager: 'zypper'
diff --git a/guests/playbooks/build/projects/libvirt-dbus.yml
b/guests/playbooks/build/projects/libvirt-dbus.yml
index 66bc1fa..e29d74b 100644
--- a/guests/playbooks/build/projects/libvirt-dbus.yml
+++ b/guests/playbooks/build/projects/libvirt-dbus.yml
@@ -20,6 +20,7 @@
- libvirt-debian-sid
- libvirt-fedora-30
- libvirt-fedora-rawhide
+ - libvirt-opensuse-151
- libvirt-ubuntu-1804
- include: '{{ playbook_base }}/jobs/meson-rpm-job.yml'
vars:
diff --git a/guests/playbooks/build/projects/libvirt-sandbox.yml
b/guests/playbooks/build/projects/libvirt-sandbox.yml
index 0b4fe50..d9e00d4 100644
--- a/guests/playbooks/build/projects/libvirt-sandbox.yml
+++ b/guests/playbooks/build/projects/libvirt-sandbox.yml
@@ -11,6 +11,7 @@
- libvirt-fedora-30
- libvirt-fedora-31
- libvirt-fedora-rawhide
+ - libvirt-opensuse-151
- libvirt-ubuntu-1604
- libvirt-ubuntu-1804
archive_format: gz
diff --git a/guests/playbooks/build/projects/virt-manager.yml
b/guests/playbooks/build/projects/virt-manager.yml
index 4b0e6dd..01f353e 100644
--- a/guests/playbooks/build/projects/virt-manager.yml
+++ b/guests/playbooks/build/projects/virt-manager.yml
@@ -13,6 +13,7 @@
- libvirt-freebsd-11
- libvirt-freebsd-12
- libvirt-freebsd-current
+ - libvirt-opensuse-151
- libvirt-ubuntu-1804
archive_format: gz
git_url: '{{ git_urls["virt-manager"][git_remote] }}'
@@ -35,6 +36,7 @@
- libvirt-freebsd-11
- libvirt-freebsd-12
- libvirt-freebsd-current
+ - libvirt-opensuse-151
- libvirt-ubuntu-1804
- include: '{{ playbook_base }}/jobs/python-distutils-rpm-job.yml'
vars:
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH v3 2/3] mappings: Adjust mappings for openSUSE 15.1
Between all the adjustments done, it's worth to mention that there's no equivalent of perl-Time-HiRes packages on openSUSE as its already part of the base perl packages and that perl-generators is a Fedora/RHEL-ism used for RPM auto-dependencies (which is not present nor needed on openSUSE). Signed-off-by: Fabiano Fidêncio --- guests/vars/mappings.yml | 14 ++ 1 file changed, 14 insertions(+) diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml index 03044d0..69750ad 100644 --- a/guests/vars/mappings.yml +++ b/guests/vars/mappings.yml @@ -134,6 +134,7 @@ mappings: dbus-daemon: default: dbus Fedora: dbus-daemon +OpenSUSE: dbus-1 device-mapper: deb: libdevmapper-dev @@ -181,6 +182,7 @@ mappings: rpm: gdk-pixbuf2-devel deb: libgdk-pixbuf2.0-dev pkg: gdk-pixbuf2 +OpenSUSE: gdk-pixbuf-devel cross-policy-deb: foreign gettext: @@ -224,6 +226,7 @@ mappings: go: default: golang FreeBSD: go +OpenSUSE: go gobject-introspection: deb: libgirepository1.0-dev @@ -243,6 +246,7 @@ mappings: gtk-update-icon-cache: default: gtk-update-icon-cache +OpenSUSE: gtk3-tools Ubuntu1604: libgtk2.0-bin hal: @@ -267,6 +271,7 @@ mappings: isoinfo: default: genisoimage FreeBSD: cdrkit +OpenSUSE: mkisofs java: deb: openjdk-11-jre-headless @@ -800,6 +805,7 @@ mappings: deb: libtime-hr-perl pkg: p5-Time-HiRes rpm: perl-Time-HiRes +OpenSUSE: perl perl-XML-Twig: deb: libxml-twig-perl @@ -823,6 +829,7 @@ mappings: perl-generators: rpm: perl-generators +OpenSUSE: CentOS7: pkg-config: @@ -836,6 +843,7 @@ mappings: pulseaudio: deb: libpulse-dev rpm: pulseaudio-libs-devel +OpenSUSE: libpulse-devel cross-policy-deb: foreign python3-docutils: @@ -849,12 +857,14 @@ mappings: python3-dbus: default: python3-dbus FreeBSD: py37-dbus +OpenSUSE: python3-dbus-python CentOS7: python36-dbus python3-devel: deb: python3-dev pkg: python3 Fedora: python3-devel +OpenSUSE: python3-devel CentOS7: python36-devel cross-policy-deb: foreign @@ -867,6 +877,7 @@ mappings: python3-libxml2: default: python3-libxml2 FreeBSD: py37-libxml2 +OpenSUSE: python3-libxml2-python CentOS7: Ubuntu1604: @@ -950,6 +961,7 @@ mappings: deb: libspice-client-gtk-3.0-dev pkg: spice-gtk rpm: spice-gtk3-devel +OpenSUSE: spice-gtk-devel cross-policy-deb: foreign strace: @@ -1018,6 +1030,7 @@ mappings: xz-static: deb: liblzma-dev Fedora: xz-static +OpenSUSE: xz-static-devel cross-policy-deb: foreign yajl: @@ -1041,6 +1054,7 @@ mappings: zlib-static: deb: zlib1g-dev rpm: zlib-static +OpenSUSE: zlib-devel-static cross-policy-deb: foreign -- 2.24.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH v3 1/3] mappings: Use meson from pip on openSUSE 15.1
The meson version present on openSUSE 15.1 is too old (0.46.0) to build our projects, which require 0.49.0. Knowing that, meson from pip has to be used. Signed-off-by: Fabiano Fidêncio --- guests/vars/mappings.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml index b80a9b4..03044d0 100644 --- a/guests/vars/mappings.yml +++ b/guests/vars/mappings.yml @@ -481,6 +481,7 @@ mappings: default: meson CentOS7: Debian9: +OpenSUSE151: Ubuntu1604: Ubuntu1804: @@ -882,6 +883,7 @@ mappings: python3-pip: CentOS7: python3-pip Debian9: python3-pip +OpenSUSE151: python3-pip Ubuntu1604: python3-pip Ubuntu1804: python3-pip -- 2.24.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH v3 0/3] Add all possible projects to openSUSE 15.1
Now that openSUSE 15.1 is a thing on libvirt-jenkins-ci, let's add all possible projects to the OS, doing the needed mappings adjustments whenever needed. Changes since v2: https://www.redhat.com/archives/libvir-list/2020-January/msg00333.html - Split meson change into its specific patch; - Re-ordered the commits based on Andrea's request; - (Possibly) Fixed all the issues pointed by Andrea; Changes since v1: https://www.redhat.com/archives/libvir-list/2020-January/msg00321.html - Added libvirt-perl and updated the mappings accordingly. Fabiano Fidêncio (3): mappings: Use meson from pip on openSUSE 15.1 mappings: Adjust mappings for openSUSE 15.1 guests: Add projects to openSUSE 15.1 guests/host_vars/libvirt-opensuse-151/main.yml | 14 ++ guests/playbooks/build/projects/libvirt-dbus.yml | 1 + .../playbooks/build/projects/libvirt-sandbox.yml | 1 + guests/playbooks/build/projects/virt-manager.yml | 2 ++ guests/vars/mappings.yml | 16 5 files changed, 34 insertions(+) -- 2.24.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH v2 0/2] Add all possible projects to OpenSUSE 15.1
Now that OpenSUSE 15.1 is a thing on libvirt-jenkins-ci, let's add all possible projects to the OS, doing the needed mappings adjustments whenever needed. Changes since v1: https://www.redhat.com/archives/libvir-list/2020-January/msg00321.html - Added libvirt-perl and updated the mappings accordingly. Fabiano Fidêncio (2): guests: Add projects to OpenSUSE 15.1 mappings: Adjust mappings for OpenSUSE 15.1 guests/host_vars/libvirt-opensuse-151/main.yml | 14 ++ guests/playbooks/build/projects/virt-manager.yml | 2 ++ guests/vars/mappings.yml | 15 +++ 3 files changed, 31 insertions(+) -- 2.24.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH v2 1/2] guests: Add projects to OpenSUSE 15.1
libvirt-tck has not been added due to the following missing packages:
- perl-Config-Record
- perl-IO-Compress-Bzip2
- perl-TAP-Formatter-HTML
- perl-TAP-Formatter-JUnit
- perl-TAP-Harness-Archive
Signed-off-by: Fabiano Fidêncio
fixup! guests: Add projects to OpenSUSE 15.1
---
guests/host_vars/libvirt-opensuse-151/main.yml | 14 ++
guests/playbooks/build/projects/virt-manager.yml | 2 ++
2 files changed, 16 insertions(+)
diff --git a/guests/host_vars/libvirt-opensuse-151/main.yml
b/guests/host_vars/libvirt-opensuse-151/main.yml
index f422a9e..88d5dfd 100644
--- a/guests/host_vars/libvirt-opensuse-151/main.yml
+++ b/guests/host_vars/libvirt-opensuse-151/main.yml
@@ -1,6 +1,20 @@
---
projects:
+ - gtk-vnc
+ - libosinfo
- libvirt
+ - libvirt-dbus
+ - libvirt-glib
+ - libvirt-go
+ - libvirt-go-xml
+ - libvirt-ocaml
+ - libvirt-perl
+ - libvirt-python
+ - libvirt-sandbox
+ - osinfo-db
+ - osinfo-db-tools
+ - virt-manager
+ - virt-viewer
package_format: 'rpm'
package_manager: 'zypper'
diff --git a/guests/playbooks/build/projects/virt-manager.yml
b/guests/playbooks/build/projects/virt-manager.yml
index 4b0e6dd..01f353e 100644
--- a/guests/playbooks/build/projects/virt-manager.yml
+++ b/guests/playbooks/build/projects/virt-manager.yml
@@ -13,6 +13,7 @@
- libvirt-freebsd-11
- libvirt-freebsd-12
- libvirt-freebsd-current
+ - libvirt-opensuse-151
- libvirt-ubuntu-1804
archive_format: gz
git_url: '{{ git_urls["virt-manager"][git_remote] }}'
@@ -35,6 +36,7 @@
- libvirt-freebsd-11
- libvirt-freebsd-12
- libvirt-freebsd-current
+ - libvirt-opensuse-151
- libvirt-ubuntu-1804
- include: '{{ playbook_base }}/jobs/python-distutils-rpm-job.yml'
vars:
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH v2 2/2] mappings: Adjust mappings for OpenSUSE 15.1
For all the projects which rely on meson, the version present on OpenSUSE 15.1 is too old (0.46.0) to build our projects, which requires 0.49.0. Knowing that, pip is going to be used there. The adjustments done, per project, are: - gtk-vnc: - gdk-pixbuf: gdk-pixbuf-devel - pulseaudio: libpulse-devel - libvirt-dbus: - dbus: dbus-1 - python3-dbus: python3-dbus-python - python3-pip: python3-pip - libvirt-go: - golang: go - libvirt-perl: - perl-Time-HiRes: - Although there's not an equivalent package for perl-Time-HiRes, OpenSUSE seems to have it as part of its base perl packages. - perl-generators: - It's a Fedora/RHEL-ism used for RPM auto-dependencies, not present nor needed on OpenSUSE; - libvirt-python: - python3-devel: python3-devel - libvirt-sandbox: - zlib-static: zlib-devel-static - virt-viewer: - spice-gtk3: spice-gtk-devel - virt-manager: - gtk-update-icon-cache: - isoinfo: mkisofs - python3-libxml2: python3-libxml2-libxml Signed-off-by: Fabiano Fidêncio --- guests/vars/mappings.yml | 15 +++ 1 file changed, 15 insertions(+) diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml index b80a9b4..c942591 100644 --- a/guests/vars/mappings.yml +++ b/guests/vars/mappings.yml @@ -134,6 +134,7 @@ mappings: dbus-daemon: default: dbus Fedora: dbus-daemon +OpenSUSE: dbus-1 device-mapper: deb: libdevmapper-dev @@ -181,6 +182,7 @@ mappings: rpm: gdk-pixbuf2-devel deb: libgdk-pixbuf2.0-dev pkg: gdk-pixbuf2 +OpenSUSE: gdk-pixbuf-devel cross-policy-deb: foreign gettext: @@ -224,6 +226,7 @@ mappings: go: default: golang FreeBSD: go +OpenSUSE: go gobject-introspection: deb: libgirepository1.0-dev @@ -243,6 +246,7 @@ mappings: gtk-update-icon-cache: default: gtk-update-icon-cache +OpenSUSE: Ubuntu1604: libgtk2.0-bin hal: @@ -267,6 +271,7 @@ mappings: isoinfo: default: genisoimage FreeBSD: cdrkit +OpenSUSE: mkisofs java: deb: openjdk-11-jre-headless @@ -479,6 +484,7 @@ mappings: meson: default: meson +OpenSUSE: CentOS7: Debian9: Ubuntu1604: @@ -799,6 +805,7 @@ mappings: deb: libtime-hr-perl pkg: p5-Time-HiRes rpm: perl-Time-HiRes +OpenSUSE: perl-XML-Twig: deb: libxml-twig-perl @@ -823,6 +830,7 @@ mappings: perl-generators: rpm: perl-generators CentOS7: +OpenSUSE: pkg-config: default: pkgconf @@ -835,6 +843,7 @@ mappings: pulseaudio: deb: libpulse-dev rpm: pulseaudio-libs-devel +OpenSUSE: libpulse-devel cross-policy-deb: foreign python3-docutils: @@ -848,12 +857,14 @@ mappings: python3-dbus: default: python3-dbus FreeBSD: py37-dbus +OpenSUSE: python3-dbus-python CentOS7: python36-dbus python3-devel: deb: python3-dev pkg: python3 Fedora: python3-devel +OpenSUSE: python3-devel CentOS7: python36-devel cross-policy-deb: foreign @@ -866,6 +877,7 @@ mappings: python3-libxml2: default: python3-libxml2 FreeBSD: py37-libxml2 +OpenSUSE: python3-libxml2-python CentOS7: Ubuntu1604: @@ -880,6 +892,7 @@ mappings: CentOS7: python36-nose python3-pip: +OpenSUSE: python3-pip CentOS7: python3-pip Debian9: python3-pip Ubuntu1604: python3-pip @@ -948,6 +961,7 @@ mappings: deb: libspice-client-gtk-3.0-dev pkg: spice-gtk rpm: spice-gtk3-devel +OpenSUSE: spice-gtk-devel cross-policy-deb: foreign strace: @@ -1039,6 +1053,7 @@ mappings: zlib-static: deb: zlib1g-dev rpm: zlib-static +OpenSUSE: zlib-devel-static cross-policy-deb: foreign -- 2.24.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [perl PATCH] make Time::HiRes optional in test suite
On Wed, Jan 8, 2020 at 3:23 PM Daniel P. Berrangé wrote: > > Signed-off-by: Daniel P. Berrangé Reviewed-by: Fabiano Fidêncio -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 2/2] mappings: Adjust mappings for OpenSUSE 15.1
For all the projects which rely on meson, the version present on OpenSUSE 15.1 is too old (0.46.0) to build our projects, which requires 0.49.0. Knowing that, pip is going to be used there. The adjustments done, per project, are: - gtk-vnc: - gdk-pixbuf: gdk-pixbuf-devel - pulseaudio: libpulse-devel - libvirt-dbus: - dbus: dbus-1 - python3-dbus: python3-dbus-python - python3-pip: python3-pip - libvirt-go: - golang: go - libvirt-python: - python3-devel: python3-devel - libvirt-sandbox: - zlib-static: zlib-devel-static - virt-viewer: - spice-gtk3: spice-gtk-devel - virt-manager: - gtk-update-icon-cache: - isoinfo: mkisofs - python3-libxml2: python3-libxml2-libxml Signed-off-by: Fabiano Fidêncio --- guests/vars/mappings.yml | 13 + 1 file changed, 13 insertions(+) diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml index b80a9b4..b1e461c 100644 --- a/guests/vars/mappings.yml +++ b/guests/vars/mappings.yml @@ -134,6 +134,7 @@ mappings: dbus-daemon: default: dbus Fedora: dbus-daemon +OpenSUSE: dbus-1 device-mapper: deb: libdevmapper-dev @@ -181,6 +182,7 @@ mappings: rpm: gdk-pixbuf2-devel deb: libgdk-pixbuf2.0-dev pkg: gdk-pixbuf2 +OpenSUSE: gdk-pixbuf-devel cross-policy-deb: foreign gettext: @@ -224,6 +226,7 @@ mappings: go: default: golang FreeBSD: go +OpenSUSE: go gobject-introspection: deb: libgirepository1.0-dev @@ -243,6 +246,7 @@ mappings: gtk-update-icon-cache: default: gtk-update-icon-cache +OpenSUSE: Ubuntu1604: libgtk2.0-bin hal: @@ -267,6 +271,7 @@ mappings: isoinfo: default: genisoimage FreeBSD: cdrkit +OpenSUSE: mkisofs java: deb: openjdk-11-jre-headless @@ -479,6 +484,7 @@ mappings: meson: default: meson +OpenSUSE: CentOS7: Debian9: Ubuntu1604: @@ -835,6 +841,7 @@ mappings: pulseaudio: deb: libpulse-dev rpm: pulseaudio-libs-devel +OpenSUSE: libpulse-devel cross-policy-deb: foreign python3-docutils: @@ -848,12 +855,14 @@ mappings: python3-dbus: default: python3-dbus FreeBSD: py37-dbus +OpenSUSE: python3-dbus-python CentOS7: python36-dbus python3-devel: deb: python3-dev pkg: python3 Fedora: python3-devel +OpenSUSE: python3-devel CentOS7: python36-devel cross-policy-deb: foreign @@ -866,6 +875,7 @@ mappings: python3-libxml2: default: python3-libxml2 FreeBSD: py37-libxml2 +OpenSUSE: python3-libxml2-python CentOS7: Ubuntu1604: @@ -880,6 +890,7 @@ mappings: CentOS7: python36-nose python3-pip: +OpenSUSE: python3-pip CentOS7: python3-pip Debian9: python3-pip Ubuntu1604: python3-pip @@ -948,6 +959,7 @@ mappings: deb: libspice-client-gtk-3.0-dev pkg: spice-gtk rpm: spice-gtk3-devel +OpenSUSE: spice-gtk-devel cross-policy-deb: foreign strace: @@ -1039,6 +1051,7 @@ mappings: zlib-static: deb: zlib1g-dev rpm: zlib-static +OpenSUSE: zlib-devel-static cross-policy-deb: foreign -- 2.24.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 1/2] guests: Add projects to OpenSUSE 15.1
The following projects were not added:
- libvirt-perl:
- missing packages:
- perl-TimeHiRes;
- perl-generators;
- libvirt-tck:
- missing packages:
- perl-Config-Record
- perl-IO-Compress-Bzip2
- perl-TAP-Formatter-HTML
- perl-TAP-Formatter-JUnit
- perl-TAP-Harness-Archive
- perl-accessors
- perl-generators
Signed-off-by: Fabiano Fidêncio
fixup! guests: Add projects to OpenSUSE 15.1
---
guests/host_vars/libvirt-opensuse-151/main.yml | 13 +
guests/playbooks/build/projects/virt-manager.yml | 2 ++
2 files changed, 15 insertions(+)
diff --git a/guests/host_vars/libvirt-opensuse-151/main.yml
b/guests/host_vars/libvirt-opensuse-151/main.yml
index f422a9e..7efedcd 100644
--- a/guests/host_vars/libvirt-opensuse-151/main.yml
+++ b/guests/host_vars/libvirt-opensuse-151/main.yml
@@ -1,6 +1,19 @@
---
projects:
+ - gtk-vnc
+ - libosinfo
- libvirt
+ - libvirt-dbus
+ - libvirt-glib
+ - libvirt-go
+ - libvirt-go-xml
+ - libvirt-ocaml
+ - libvirt-python
+ - libvirt-sandbox
+ - osinfo-db
+ - osinfo-db-tools
+ - virt-manager
+ - virt-viewer
package_format: 'rpm'
package_manager: 'zypper'
diff --git a/guests/playbooks/build/projects/virt-manager.yml
b/guests/playbooks/build/projects/virt-manager.yml
index 4b0e6dd..01f353e 100644
--- a/guests/playbooks/build/projects/virt-manager.yml
+++ b/guests/playbooks/build/projects/virt-manager.yml
@@ -13,6 +13,7 @@
- libvirt-freebsd-11
- libvirt-freebsd-12
- libvirt-freebsd-current
+ - libvirt-opensuse-151
- libvirt-ubuntu-1804
archive_format: gz
git_url: '{{ git_urls["virt-manager"][git_remote] }}'
@@ -35,6 +36,7 @@
- libvirt-freebsd-11
- libvirt-freebsd-12
- libvirt-freebsd-current
+ - libvirt-opensuse-151
- libvirt-ubuntu-1804
- include: '{{ playbook_base }}/jobs/python-distutils-rpm-job.yml'
vars:
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 0/2] Add all possible projects to OpenSUSE 15.1
Now that OpenSUSE 15.1 is a thing on libvirt-jenkins-ci, let's add all possible projects to the OS, doing the needed mappings adjustments whenever needed. Fabiano Fidêncio (2): guests: Add projects to OpenSUSE 15.1 mappings: Adjust mappings for OpenSUSE 15.1 guests/host_vars/libvirt-opensuse-151/main.yml | 13 + guests/playbooks/build/projects/virt-manager.yml | 2 ++ guests/vars/mappings.yml | 13 + 3 files changed, 28 insertions(+) -- 2.24.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add overrides for network port UUID getter/lookup methods
[snip]
> +static PyObject *
> +libvirt_virNetworkPortLookupByUUID(PyObject *self ATTRIBUTE_UNUSED,
> + PyObject *args)
> +{
> +virNetworkPortPtr c_retval;
> +virNetworkPtr net;
> +PyObject *pyobj_net;
> +unsigned char *uuid;
> +int len;
> +
> +if (!PyArg_ParseTuple(args, (char *)"Oz#:virNetworkPortLookupByUUID",
> + _net, , ))
> +return NULL;
> +net = (virNetworkPtr) PyvirNetwork_Get(pyobj_net);
> +
Shouldn't we also check whether net is NULL here?
> +if ((uuid == NULL) || (len != VIR_UUID_BUFLEN))
> +return VIR_PY_NONE;
> +
> +LIBVIRT_BEGIN_ALLOW_THREADS;
> +c_retval = virNetworkPortLookupByUUID(net, uuid);
> +LIBVIRT_END_ALLOW_THREADS;
> +
> +return libvirt_virNetworkPortPtrWrap((virNetworkPortPtr) c_retval);
> +}
> +
[snip]
With that fixed, Reviewed-by: Fabiano Fidêncio
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 13/23] src: replace last_component() with g_path_get_basename()
[snip]
> diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
> index f072afe857..16a527e399 100644
> --- a/src/rpc/virnetsocket.c
> +++ b/src/rpc/virnetsocket.c
> @@ -55,7 +55,6 @@
> #include "virprobe.h"
> #include "virprocess.h"
> #include "virstring.h"
> -#include "dirname.h"
> #include "passfd.h"
>
> #if WITH_SSH2
> @@ -668,7 +667,7 @@ int virNetSocketNewConnectUNIX(const char *path,
> remoteAddr.len = sizeof(remoteAddr.data.un);
>
> if (spawnDaemon) {
> -const char *binname;
> +g_autofree char *binname = NULL;
>
> if (spawnDaemon && !binary) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> @@ -677,7 +676,7 @@ int virNetSocketNewConnectUNIX(const char *path,
> goto cleanup;
> }
>
> -if (!(binname = last_component(binary)) || binname[0] == '\0') {
> +if (!(binname = g_path_get_basename(binary)) || binname[0] == '\0') {
IIUC, this check is no longer valid.
According to the g_path_get_basename() documentation "If file_name
ends with a directory separator it gets the component before the last
slash. If file_name consists only of directory separators (and on
Windows, possibly a drive letter), a single separator is returned. If
file_name is empty, it gets "."."
Knowing that, shouldn't we adapt the check accordingly?
[snip]
> diff --git a/src/util/virmdev.c b/src/util/virmdev.c
> index cd52a91ffd..c2499c0a20 100644
> --- a/src/util/virmdev.c
> +++ b/src/util/virmdev.c
> @@ -18,7 +18,6 @@
>
> #include
>
> -#include "dirname.h"
> #include "virmdev.h"
> #include "virlog.h"
> #include "virerror.h"
> @@ -207,6 +206,7 @@ char *
> virMediatedDeviceGetIOMMUGroupDev(const char *uuidstr)
> {
> g_autofree char *result_path = NULL;
> +g_autofree char *result_file = NULL;
> g_autofree char *iommu_path = NULL;
> g_autofree char *dev_path = virMediatedDeviceGetSysfsPath(uuidstr);
> char *vfio_path = NULL;
> @@ -226,7 +226,9 @@ virMediatedDeviceGetIOMMUGroupDev(const char *uuidstr)
> return NULL;
> }
>
> -vfio_path = g_strdup_printf("/dev/vfio/%s", last_component(result_path));
> +result_file = g_path_get_basename(result_path);
> +
> +vfio_path = g_strdup_printf("/dev/vfio/%s", result_file);
Please, while changing it, use g_build_filename() instead of g_strdup_printf().
[snip]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 19/23] util: replace gethostname() with g_get_hostname()
[snip]
> -r = gethostname(virLogHostname, sizeof(virLogHostname));
> -if (r == -1) {
> -ignore_value(virStrcpyStatic(virLogHostname, "(unknown)"));
> -} else {
> -NUL_TERMINATE(virLogHostname);
> -}
> +(void)g_get_host_name();
Why not use ignore_return() here?
[snip]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/23] Take 64 gnulib modules, eliminate 14, to give 50 remaining
Daniel, On Thu, Jan 2, 2020 at 3:59 PM Daniel P. Berrangé wrote: > > In the last days before the xmas break I took some time to > eliminate 14 more gnulib modules, bringing us down to just > 50 left to go. They're getting harder to eliminate as we go > on, but to give some hints, I've annotated every module in > bootstrap.conf with a suggested replacement strategy. > > Daniel P. Berrangé (23): > build: set min version for CLang to 3.4 / XCode CLang to 5.1 > docs: expand macOS platform support coverage > travis: add macOS Xcode 11.3 testing > util: add note about event file descriptors on Windows > src: always pull in glib/gstdio.h header > src: switch to use g_setenv/g_unsetenv In the patch above I'd also take the opportunity and use TRUE/FALSE instead of 1/0 in g_setenv(). Feel free to ignore as this is just a minor nitpick; > util: add compat wrapper for g_fsync > src: use g_fsync for portability > util: introduce virFileDataSync What would be the impact of using g_fsync() on Linuxes as well? > src: use g_lstat() instead of lstat() > src: switch from fnmatch to g_pattern_match_simple This one worries me a little bit about possible breakages, mainly related to wildcards used in libvirtd.conf. > src: replace clock_gettime()/gettimeofday() with g_get_real_time() > src: replace last_component() with g_path_get_basename() Please, take a look at this patch's reply. > util: replace IS_ABSOLUTE_FILE_NAME with g_path_is_absolute > src: replace mdir_name() with g_path_get_dirname() > src: remove unused imports of dirname.h > src: replace getcwd() with g_get_current_dir() > util: use realpath/g_canonicalize_filename > util: replace gethostname() with g_get_hostname() Please, take a look at this patch's reply. > src: replace WSAStartup with g_networking_init() > src: replace strptime()/timegm()/mktime() with GDateTime APIs set > bootstrap: remove now unused gnulib modules > bootstrap: annotate with info about desired replacement > All the comments are more suggestions than blockers. The questions are not blockers in any way. Knowing that, for the series: Reviewed-by: Fabiano Fidêncio [snip] -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/5] util: Use g_auto in virFirewallStartTransaction()
Signed-off-by: Fabiano Fidêncio
---
src/util/virfirewall.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 2177617ecf..564e2fe0be 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -278,6 +278,7 @@ virFirewallGroupFree(virFirewallGroupPtr group)
VIR_FREE(group);
}
+G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virFirewallGroupPtr, virFirewallGroupFree,
NULL);
/**
* virFirewallFree:
@@ -575,7 +576,7 @@ size_t virFirewallRuleGetArgCount(virFirewallRulePtr rule)
void virFirewallStartTransaction(virFirewallPtr firewall,
unsigned int flags)
{
-virFirewallGroupPtr group;
+g_auto(virFirewallGroupPtr) group = NULL;
VIR_FIREWALL_RETURN_IF_ERROR(firewall);
@@ -586,10 +587,9 @@ void virFirewallStartTransaction(virFirewallPtr firewall,
if (VIR_EXPAND_N(firewall->groups,
firewall->ngroups, 1) < 0) {
-virFirewallGroupFree(group);
return;
}
-firewall->groups[firewall->ngroups - 1] = group;
+firewall->groups[firewall->ngroups - 1] = g_steal_pointer();
firewall->currentGroup = firewall->ngroups - 1;
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/5] util: Don't set/check for ENOMEM as a firewall error
As libvirt decided to take the path to not report OOM and simply abort
when it happens, let's not set nor check for ENOMEM firewall's error and
simplify the code whenever it's possible.
Signed-off-by: Fabiano Fidêncio
---
src/util/virfirewall.c | 30 ++
1 file changed, 6 insertions(+), 24 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index ee72b579e4..6f7b5306e5 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -391,7 +391,6 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
return rule;
no_memory:
-firewall->err = ENOMEM;
virFirewallRuleFree(rule);
return NULL;
}
@@ -492,10 +491,8 @@ void virFirewallRuleAddArg(virFirewallPtr firewall,
ADD_ARG(rule, arg);
-return;
-
no_memory:
-firewall->err = ENOMEM;
+return;
}
@@ -514,10 +511,8 @@ void virFirewallRuleAddArgFormat(virFirewallPtr firewall,
ADD_ARG(rule, arg);
-return;
-
no_memory:
-firewall->err = ENOMEM;
+return;
}
@@ -532,10 +527,8 @@ void virFirewallRuleAddArgSet(virFirewallPtr firewall,
args++;
}
-return;
-
no_memory:
-firewall->err = ENOMEM;
+return;
}
@@ -553,12 +546,7 @@ void virFirewallRuleAddArgList(virFirewallPtr firewall,
while ((str = va_arg(list, char *)) != NULL)
ADD_ARG(rule, str);
-va_end(list);
-
-return;
-
no_memory:
-firewall->err = ENOMEM;
va_end(list);
}
@@ -591,15 +579,13 @@ void virFirewallStartTransaction(virFirewallPtr firewall,
VIR_FIREWALL_RETURN_IF_ERROR(firewall);
-if (!(group = virFirewallGroupNew())) {
-firewall->err = ENOMEM;
+if (!(group = virFirewallGroupNew()))
return;
-}
+
group->actionFlags = flags;
if (VIR_EXPAND_N(firewall->groups,
firewall->ngroups, 1) < 0) {
-firewall->err = ENOMEM;
virFirewallGroupFree(group);
return;
}
@@ -747,10 +733,6 @@ virFirewallApplyRule(virFirewallPtr firewall,
if (rule->queryCB(firewall, rule->layer, (const char *const *)lines,
rule->queryOpaque) < 0)
return -1;
-if (firewall->err == ENOMEM) {
-virReportOOMError();
-return -1;
-}
if (firewall->err) {
virReportSystemError(firewall->err, "%s",
_("Unable to create rule"));
@@ -818,7 +800,7 @@ virFirewallApply(virFirewallPtr firewall)
_("Failed to initialize a valid firewall backend"));
goto cleanup;
}
-if (!firewall || firewall->err == ENOMEM) {
+if (!firewall) {
virReportOOMError();
goto cleanup;
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 5/5] docs: Remove mention to no_memory label
no_memory labels have been entirely removed from libvirt code base. Knowing that, let's also remove any mention to the label from our hacking guide. Signed-off-by: Fabiano Fidêncio --- docs/hacking.html.in | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 74aba5d46b..90bd0ddc81 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -1491,7 +1491,6 @@ BAD: error: A path only taken upon return with an error code cleanup: A path taken upon return with success code + optional error - no_memory: A path only taken upon return with an OOM error code retry: If needing to jump upwards (e.g., retry on EINTR) -- 2.24.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/5] util: Adapt ADD_ARG() macro
As VIR_RESIZE_N() macro already aborts in case of OOM, there's no reason
to check for its output in the ADD_ARG() macro.
By doing this, we can simply get rid of a all no_memory labels spread in
the virfirewall.c file.
Signed-off-by: Fabiano Fidêncio
---
src/util/virfirewall.c | 21 +++--
1 file changed, 3 insertions(+), 18 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 564e2fe0be..7c8040880c 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -320,11 +320,9 @@ void virFirewallFree(virFirewallPtr firewall)
#define ADD_ARG(rule, str) \
do { \
-if (VIR_RESIZE_N(rule->args, \
- rule->argsAlloc, \
- rule->argsLen, 1) < 0) \
-goto no_memory; \
- \
+ignore_value(VIR_RESIZE_N(rule->args, \
+ rule->argsAlloc, \
+ rule->argsLen, 1)); \
rule->args[rule->argsLen++] = g_strdup(str); \
} while (0)
@@ -391,9 +389,6 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
return g_steal_pointer();
-
- no_memory:
-return NULL;
}
@@ -491,9 +486,6 @@ void virFirewallRuleAddArg(virFirewallPtr firewall,
VIR_FIREWALL_RULE_RETURN_IF_ERROR(firewall, rule);
ADD_ARG(rule, arg);
-
- no_memory:
-return;
}
@@ -511,9 +503,6 @@ void virFirewallRuleAddArgFormat(virFirewallPtr firewall,
va_end(list);
ADD_ARG(rule, arg);
-
- no_memory:
-return;
}
@@ -527,9 +516,6 @@ void virFirewallRuleAddArgSet(virFirewallPtr firewall,
ADD_ARG(rule, *args);
args++;
}
-
- no_memory:
-return;
}
@@ -547,7 +533,6 @@ void virFirewallRuleAddArgList(virFirewallPtr firewall,
while ((str = va_arg(list, char *)) != NULL)
ADD_ARG(rule, str);
- no_memory:
va_end(list);
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/5] util: Use g_auto/g_autofree in virFirewallAddRuleFullV()
Signed-off-by: Fabiano Fidêncio
---
src/util/virfirewall.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 6f7b5306e5..2177617ecf 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -257,6 +257,7 @@ virFirewallRuleFree(virFirewallRulePtr rule)
VIR_FREE(rule);
}
+G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virFirewallRulePtr, virFirewallRuleFree, NULL);
static void
virFirewallGroupFree(virFirewallGroupPtr group)
@@ -335,8 +336,8 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
va_list args)
{
virFirewallGroupPtr group;
-virFirewallRulePtr rule;
-char *str;
+g_auto(virFirewallRulePtr) rule = NULL;
+g_autofree char *str = NULL;
VIR_FIREWALL_RETURN_NULL_IF_ERROR(firewall);
@@ -348,7 +349,7 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
if (VIR_ALLOC(rule) < 0)
-goto no_memory;
+return NULL;
rule->layer = layer;
rule->queryCB = cb;
@@ -379,19 +380,18 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
if (VIR_APPEND_ELEMENT_COPY(group->rollback,
group->nrollback,
rule) < 0)
-goto no_memory;
+return NULL;
} else {
if (VIR_APPEND_ELEMENT_COPY(group->action,
group->naction,
rule) < 0)
-goto no_memory;
+return NULL;
}
-return rule;
+return g_steal_pointer();
no_memory:
-virFirewallRuleFree(rule);
return NULL;
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/5] Remove the last "no_memory" label
This series touches virfirewall.c, the last place where a no_memory label can be found. The series: - Gets rid of setting and checking for ENOMEM as a firewall's error; - Use g_auto / g_autofree in a few different places; - Adapt ADD_ARG() macro as VIR_RESIZE_N() just aborts in case of OOM; - Remove no_memory mention from the hacking guide; Changes since v1: https://www.redhat.com/archives/libvir-list/2020-January/msg00018.html - Daniel suggested to get rid of ADD_ARG(). Instead of doing so, I've adapted the macro (as it does more than just a realloc) and, by doing so, got rid of the no_memory labels; - Dropped: - util: Rename 'no_memory' label to 'cleanup' - util: Add ADD_ARG_RETURN_ON_ERROR - util: Add ADD_ARG_RETURN_VALUE_ON_ERROR - Added: - util: Adapt ADD_ARG() macro Fabiano Fidêncio (5): util: Don't set/check for ENOMEM as a firewall error util: Use g_auto/g_autofree in virFirewallAddRuleFullV() util: Use g_auto in virFirewallStartTransaction() util: Adapt ADD_ARG() macro docs: Remove mention to no_memory label docs/hacking.html.in | 1 - src/util/virfirewall.c | 65 +++--- 2 files changed, 16 insertions(+), 50 deletions(-) -- 2.24.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/7] util: Add ADD_ARG_RETURN_ON_ERROR
[snip]
> > +#define ADD_ARG_RETURN_ON_ERROR(rule, str) \
> > +do { \
> > +if (VIR_RESIZE_N(rule->args, \
> > + rule->argsAlloc, \
> > + rule->argsLen, 1) < 0) \
> > +return; \
> > + \
> > +rule->args[rule->argsLen++] = g_strdup(str); \
> > +} while (0)
>
> IMHO this is missing the benefit of using glib since VIR_RESIZE_N
> will never fail now. We should get rid of "ADD_ARG" entirely, and
> just put a g_realloc() call directly in the place it is needed
> without any macros. Likewise for the next patch.
Cool, I really missed that.
I'll submit a v2, soon.
Best Regards,
--
Fabiano Fidêncio.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 7/7] docs: Remove mention to no_memory label
no_memory labels have been entirely removed from libvirt codebase. Knowing that, let's also remove any mention to the label from our hacking guide. Signed-off-by: Fabiano Fidêncio --- docs/hacking.html.in | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 74aba5d46b..90bd0ddc81 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -1491,7 +1491,6 @@ BAD: error: A path only taken upon return with an error code cleanup: A path taken upon return with success code + optional error - no_memory: A path only taken upon return with an OOM error code retry: If needing to jump upwards (e.g., retry on EINTR) -- 2.24.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/7] util: Use g_auto in virFirewallStartTransaction()
Signed-off-by: Fabiano Fidêncio
---
src/util/virfirewall.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index d6fff31318..6dace18bc4 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -278,6 +278,7 @@ virFirewallGroupFree(virFirewallGroupPtr group)
VIR_FREE(group);
}
+G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virFirewallGroupPtr, virFirewallGroupFree,
NULL);
/**
* virFirewallFree:
@@ -575,7 +576,7 @@ size_t virFirewallRuleGetArgCount(virFirewallRulePtr rule)
void virFirewallStartTransaction(virFirewallPtr firewall,
unsigned int flags)
{
-virFirewallGroupPtr group;
+g_auto(virFirewallGroupPtr) group = NULL;
VIR_FIREWALL_RETURN_IF_ERROR(firewall);
@@ -586,10 +587,9 @@ void virFirewallStartTransaction(virFirewallPtr firewall,
if (VIR_EXPAND_N(firewall->groups,
firewall->ngroups, 1) < 0) {
-virFirewallGroupFree(group);
return;
}
-firewall->groups[firewall->ngroups - 1] = group;
+firewall->groups[firewall->ngroups - 1] = g_steal_pointer();
firewall->currentGroup = firewall->ngroups - 1;
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/7] util: Rename 'no_memory' label to 'cleanup'
Let's rename the no_memory label to cleanup as the first step to remove
the OOM handling in virfirewall.c file.
Signed-off-by: Fabiano Fidêncio
---
src/util/virfirewall.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index ee72b579e4..c6a5de8842 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -321,7 +321,7 @@ void virFirewallFree(virFirewallPtr firewall)
if (VIR_RESIZE_N(rule->args, \
rule->argsAlloc, \
rule->argsLen, 1) < 0) \
-goto no_memory; \
+goto cleanup; \
\
rule->args[rule->argsLen++] = g_strdup(str); \
} while (0)
@@ -348,7 +348,7 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
if (VIR_ALLOC(rule) < 0)
-goto no_memory;
+goto cleanup;
rule->layer = layer;
rule->queryCB = cb;
@@ -379,18 +379,18 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
if (VIR_APPEND_ELEMENT_COPY(group->rollback,
group->nrollback,
rule) < 0)
-goto no_memory;
+goto cleanup;
} else {
if (VIR_APPEND_ELEMENT_COPY(group->action,
group->naction,
rule) < 0)
-goto no_memory;
+goto cleanup;
}
return rule;
- no_memory:
+ cleanup:
firewall->err = ENOMEM;
virFirewallRuleFree(rule);
return NULL;
@@ -494,7 +494,7 @@ void virFirewallRuleAddArg(virFirewallPtr firewall,
return;
- no_memory:
+ cleanup:
firewall->err = ENOMEM;
}
@@ -516,7 +516,7 @@ void virFirewallRuleAddArgFormat(virFirewallPtr firewall,
return;
- no_memory:
+ cleanup:
firewall->err = ENOMEM;
}
@@ -534,7 +534,7 @@ void virFirewallRuleAddArgSet(virFirewallPtr firewall,
return;
- no_memory:
+ cleanup:
firewall->err = ENOMEM;
}
@@ -557,7 +557,7 @@ void virFirewallRuleAddArgList(virFirewallPtr firewall,
return;
- no_memory:
+ cleanup:
firewall->err = ENOMEM;
va_end(list);
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/7] util: Add ADD_ARG_RETURN_ON_ERROR
Similarly to ADD_ARG, let's create an ADD_ARG_RETURN_ON_ERROR macro
which will simply return instead of going to a cleanup label.
By doing this, we can get rid of a few cleanup labels spread in the
code.
Signed-off-by: Fabiano Fidêncio
---
src/util/virfirewall.c | 25 +
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 6dace18bc4..d632f72502 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -328,6 +328,16 @@ void virFirewallFree(virFirewallPtr firewall)
rule->args[rule->argsLen++] = g_strdup(str); \
} while (0)
+#define ADD_ARG_RETURN_ON_ERROR(rule, str) \
+do { \
+if (VIR_RESIZE_N(rule->args, \
+ rule->argsAlloc, \
+ rule->argsLen, 1) < 0) \
+return; \
+ \
+rule->args[rule->argsLen++] = g_strdup(str); \
+} while (0)
+
static virFirewallRulePtr
virFirewallAddRuleFullV(virFirewallPtr firewall,
virFirewallLayer layer,
@@ -490,10 +500,7 @@ void virFirewallRuleAddArg(virFirewallPtr firewall,
{
VIR_FIREWALL_RULE_RETURN_IF_ERROR(firewall, rule);
-ADD_ARG(rule, arg);
-
- cleanup:
-return;
+ADD_ARG_RETURN_ON_ERROR(rule, arg);
}
@@ -510,10 +517,7 @@ void virFirewallRuleAddArgFormat(virFirewallPtr firewall,
arg = g_strdup_vprintf(fmt, list);
va_end(list);
-ADD_ARG(rule, arg);
-
- cleanup:
-return;
+ADD_ARG_RETURN_ON_ERROR(rule, arg);
}
@@ -524,12 +528,9 @@ void virFirewallRuleAddArgSet(virFirewallPtr firewall,
VIR_FIREWALL_RULE_RETURN_IF_ERROR(firewall, rule);
while (*args) {
-ADD_ARG(rule, *args);
+ADD_ARG_RETURN_ON_ERROR(rule, *args);
args++;
}
-
- cleanup:
-return;
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/7] util: Don't set/check for ENOMEM as a firewall error
As libvirt decided to take the path to not report OOM and simply abort
when it happens, let's get rid of OOM treatment in virfirewall.c and
simplify the code whenever it's possible.
Signed-off-by: Fabiano Fidêncio
---
src/util/virfirewall.c | 30 ++
1 file changed, 6 insertions(+), 24 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index c6a5de8842..00b1bd0a97 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -391,7 +391,6 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
return rule;
cleanup:
-firewall->err = ENOMEM;
virFirewallRuleFree(rule);
return NULL;
}
@@ -492,10 +491,8 @@ void virFirewallRuleAddArg(virFirewallPtr firewall,
ADD_ARG(rule, arg);
-return;
-
cleanup:
-firewall->err = ENOMEM;
+return;
}
@@ -514,10 +511,8 @@ void virFirewallRuleAddArgFormat(virFirewallPtr firewall,
ADD_ARG(rule, arg);
-return;
-
cleanup:
-firewall->err = ENOMEM;
+return;
}
@@ -532,10 +527,8 @@ void virFirewallRuleAddArgSet(virFirewallPtr firewall,
args++;
}
-return;
-
cleanup:
-firewall->err = ENOMEM;
+return;
}
@@ -553,12 +546,7 @@ void virFirewallRuleAddArgList(virFirewallPtr firewall,
while ((str = va_arg(list, char *)) != NULL)
ADD_ARG(rule, str);
-va_end(list);
-
-return;
-
cleanup:
-firewall->err = ENOMEM;
va_end(list);
}
@@ -591,15 +579,13 @@ void virFirewallStartTransaction(virFirewallPtr firewall,
VIR_FIREWALL_RETURN_IF_ERROR(firewall);
-if (!(group = virFirewallGroupNew())) {
-firewall->err = ENOMEM;
+if (!(group = virFirewallGroupNew()))
return;
-}
+
group->actionFlags = flags;
if (VIR_EXPAND_N(firewall->groups,
firewall->ngroups, 1) < 0) {
-firewall->err = ENOMEM;
virFirewallGroupFree(group);
return;
}
@@ -747,10 +733,6 @@ virFirewallApplyRule(virFirewallPtr firewall,
if (rule->queryCB(firewall, rule->layer, (const char *const *)lines,
rule->queryOpaque) < 0)
return -1;
-if (firewall->err == ENOMEM) {
-virReportOOMError();
-return -1;
-}
if (firewall->err) {
virReportSystemError(firewall->err, "%s",
_("Unable to create rule"));
@@ -818,7 +800,7 @@ virFirewallApply(virFirewallPtr firewall)
_("Failed to initialize a valid firewall backend"));
goto cleanup;
}
-if (!firewall || firewall->err == ENOMEM) {
+if (!firewall) {
virReportOOMError();
goto cleanup;
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/7] util: Use g_auto/g_autofree in virFirewallAddRuleFullV()
Signed-off-by: Fabiano Fidêncio
---
src/util/virfirewall.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 00b1bd0a97..d6fff31318 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -257,6 +257,7 @@ virFirewallRuleFree(virFirewallRulePtr rule)
VIR_FREE(rule);
}
+G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virFirewallRulePtr, virFirewallRuleFree, NULL);
static void
virFirewallGroupFree(virFirewallGroupPtr group)
@@ -335,8 +336,8 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
va_list args)
{
virFirewallGroupPtr group;
-virFirewallRulePtr rule;
-char *str;
+g_auto(virFirewallRulePtr) rule = NULL;
+g_autofree char *str = NULL;
VIR_FIREWALL_RETURN_NULL_IF_ERROR(firewall);
@@ -348,7 +349,7 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
if (VIR_ALLOC(rule) < 0)
-goto cleanup;
+return NULL;
rule->layer = layer;
rule->queryCB = cb;
@@ -379,19 +380,18 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
if (VIR_APPEND_ELEMENT_COPY(group->rollback,
group->nrollback,
rule) < 0)
-goto cleanup;
+return NULL;
} else {
if (VIR_APPEND_ELEMENT_COPY(group->action,
group->naction,
rule) < 0)
-goto cleanup;
+return NULL;
}
-return rule;
+return g_steal_pointer();
cleanup:
-virFirewallRuleFree(rule);
return NULL;
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/7] Remove the last "no_memory" label
This series touches virfirewall.c, the last place where a no_memory label can be found. The series: - Renames no_memory to cleanup; - Gets rid of ENOMEM error treatment; - Use g_auto / g_autofree in a few different places; - Add new macros as ADD_ARG heavily rely on a goto; - Remove no_memory mention from the hacking guide; I'm not totally sure whether the new macros addition are worth it. Fabiano Fidêncio (7): util: Rename 'no_memory' label to 'cleanup' util: Don't set/check for ENOMEM as a firewall error util: Use g_auto/g_autofree in virFirewallAddRuleFullV() util: Use g_auto in virFirewallStartTransaction() util: Add ADD_ARG_RETURN_ON_ERROR util: Add ADD_ARG_RETURN_VALUE_ON_ERROR docs: Remove mention to no_memory label docs/hacking.html.in | 1 - src/util/virfirewall.c | 94 +++--- 2 files changed, 42 insertions(+), 53 deletions(-) -- 2.24.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/7] util: Add ADD_ARG_RETURN_VALUE_ON_ERROR
Similarly to ADD_ARG, let's create an ADD_ARG_RETURN_VALUE_ON_ERROR
macro which will simply return a value instead of going to a cleanup
label.
This patch will get rid of the cleanup label present in
virFirewallAddRuleFullV().
Signed-off-by: Fabiano Fidêncio
---
src/util/virfirewall.c | 21 ++---
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index d632f72502..1b04fc5ee4 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -338,6 +338,16 @@ void virFirewallFree(virFirewallPtr firewall)
rule->args[rule->argsLen++] = g_strdup(str); \
} while (0)
+#define ADD_ARG_RETURN_VALUE_ON_ERROR(rule, str, value) \
+do { \
+if (VIR_RESIZE_N(rule->args, \
+ rule->argsAlloc, \
+ rule->argsLen, 1) < 0) \
+return value; \
+ \
+rule->args[rule->argsLen++] = g_strdup(str); \
+} while (0)
+
static virFirewallRulePtr
virFirewallAddRuleFullV(virFirewallPtr firewall,
virFirewallLayer layer,
@@ -370,22 +380,22 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
switch (rule->layer) {
case VIR_FIREWALL_LAYER_ETHERNET:
if (ebtablesUseLock)
-ADD_ARG(rule, "--concurrent");
+ADD_ARG_RETURN_VALUE_ON_ERROR(rule, "--concurrent", NULL);
break;
case VIR_FIREWALL_LAYER_IPV4:
if (iptablesUseLock)
-ADD_ARG(rule, "-w");
+ADD_ARG_RETURN_VALUE_ON_ERROR(rule, "-w", NULL);
break;
case VIR_FIREWALL_LAYER_IPV6:
if (ip6tablesUseLock)
-ADD_ARG(rule, "-w");
+ADD_ARG_RETURN_VALUE_ON_ERROR(rule, "-w", NULL);
break;
case VIR_FIREWALL_LAYER_LAST:
break;
}
while ((str = va_arg(args, char *)) != NULL)
-ADD_ARG(rule, str);
+ADD_ARG_RETURN_VALUE_ON_ERROR(rule, str, NULL);
if (group->addingRollback) {
if (VIR_APPEND_ELEMENT_COPY(group->rollback,
@@ -401,9 +411,6 @@ virFirewallAddRuleFullV(virFirewallPtr firewall,
return g_steal_pointer();
-
- cleanup:
-return NULL;
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] rpc: Get rid of "no_memory" labels
As pointed out by Ján Tomko, "no_memory seems suspicious in the times of
abort()".
As libvirt decided to take the path to not report OOM and simply abort
when it happens, let's get rid of the no_memory labels and simplify the
code around them.
Signed-off-by: Fabiano Fidêncio
---
src/rpc/virnetclient.c | 42 --
1 file changed, 12 insertions(+), 30 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index eba8b865d1..50489b754c 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -439,7 +439,6 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
virURIPtr uri)
{
virNetSocketPtr sock = NULL;
-virNetClientPtr ret = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
g_autofree char *nc = NULL;
@@ -457,7 +456,7 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
confdir = virGetUserConfigDirectory();
virBufferAsprintf(, "%s/known_hosts", confdir);
if (!(knownhosts = virBufferContentAndReset()))
-goto no_memory;
+return NULL;
}
if (privkeyPath) {
@@ -465,7 +464,7 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
} else {
homedir = virGetUserDirectory();
if (virNetClientFindDefaultSshKey(homedir, ) < 0)
-goto no_memory;
+return NULL;
}
if (!authMethods) {
@@ -483,11 +482,11 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
virBufferEscapeShell(, netcatPath);
if (!(nc = virBufferContentAndReset()))
-goto no_memory;
+return NULL;
virBufferEscapeShell(, nc);
VIR_FREE(nc);
if (!(nc = virBufferContentAndReset()))
-goto no_memory;
+return NULL;
virBufferAsprintf(,
"sh -c "
@@ -500,24 +499,16 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
nc, nc, socketPath);
if (!(command = virBufferContentAndReset()))
-goto no_memory;
+return NULL;
if (virNetSocketNewConnectLibSSH2(host, port,
family,
username, privkey,
knownhosts, knownHostsVerify,
authMethods,
command, authPtr, uri, ) != 0)
-goto cleanup;
-
-if (!(ret = virNetClientNew(sock, NULL)))
-goto cleanup;
-
- cleanup:
-return ret;
+return NULL;
- no_memory:
-virReportOOMError();
-goto cleanup;
+ return virNetClientNew(sock, NULL);
}
#undef DEFAULT_VALUE
@@ -538,7 +529,6 @@ virNetClientPtr virNetClientNewLibssh(const char *host,
virURIPtr uri)
{
virNetSocketPtr sock = NULL;
-virNetClientPtr ret = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
g_autofree char *nc = NULL;
@@ -562,7 +552,7 @@ virNetClientPtr virNetClientNewLibssh(const char *host,
} else {
homedir = virGetUserDirectory();
if (virNetClientFindDefaultSshKey(homedir, ) < 0)
-goto no_memory;
+return NULL;
}
if (!authMethods) {
@@ -580,11 +570,11 @@ virNetClientPtr virNetClientNewLibssh(const char *host,
virBufferEscapeShell(, netcatPath);
if (!(nc = virBufferContentAndReset()))
-goto no_memory;
+return NULL;
virBufferEscapeShell(, nc);
VIR_FREE(nc);
if (!(nc = virBufferContentAndReset()))
-goto no_memory;
+return NULL;
command = g_strdup_printf("sh -c "
"'if '%s' -q 2>&1 | grep \"requires an
argument\" >/dev/null 2>&1; then "
@@ -596,17 +586,9 @@ virNetClientPtr virNetClientNewLibssh(const char *host,
username, privkey,
knownhosts, knownHostsVerify, authMethods,
command, authPtr, uri, ) != 0)
-goto cleanup;
-
-if (!(ret = virNetClientNew(sock, NULL)))
-goto cleanup;
-
- cleanup:
-return ret;
+return NULL;
- no_memory:
-virReportOOMError();
-goto cleanup;
+return virNetClientNew(sock, NULL);
}
#undef DEFAULT_VALUE
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] util: Get rid of "no_memory" labels
As pointed out by Ján Tomko, "no_memory seems suspicious in the times of
abort()".
As libvirt decided to take the path to not report OOM and simply abort
when it happens, let's get rid of the no_memory labels and simplify the
code around them.
Mind that virfirewall.c was not touched and still contains no_memory
labels. The reason those are left behind, at least for now, is because
the conversion seems to be slightly more complicated than the rest, as
some other places are relying on firewall->err being set to ENOMEM.
Signed-off-by: Fabiano Fidêncio
---
src/util/virsysinfo.c | 64 ---
src/util/virsysinfo.h | 2 ++
src/util/viruri.c | 28 +++
3 files changed, 35 insertions(+), 59 deletions(-)
diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c
index 8132ab7a07..e5af4f25e0 100644
--- a/src/util/virsysinfo.c
+++ b/src/util/virsysinfo.c
@@ -302,33 +302,27 @@ virSysinfoParsePPCProcessor(const char *base,
virSysinfoDefPtr ret)
virSysinfoDefPtr
virSysinfoReadPPC(void)
{
-virSysinfoDefPtr ret = NULL;
-char *outbuf = NULL;
+g_auto(virSysinfoDefPtr) ret = NULL;
+g_autofree char *outbuf = NULL;
if (VIR_ALLOC(ret) < 0)
-goto no_memory;
+return NULL;
if (virFileReadAll(CPUINFO, CPUINFO_FILE_LEN, ) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to open %s"), CPUINFO);
-goto no_memory;
+return NULL;
}
ret->nprocessor = 0;
ret->processor = NULL;
if (virSysinfoParsePPCProcessor(outbuf, ret) < 0)
-goto no_memory;
+return NULL;
if (virSysinfoParsePPCSystem(outbuf, >system) < 0)
-goto no_memory;
-
-VIR_FREE(outbuf);
-return ret;
+return NULL;
- no_memory:
-VIR_FREE(outbuf);
-virSysinfoDefFree(ret);
-return NULL;
+return g_steal_pointer();
}
@@ -433,13 +427,13 @@ virSysinfoParseARMProcessor(const char *base,
virSysinfoDefPtr ret)
virSysinfoDefPtr
virSysinfoReadARM(void)
{
-virSysinfoDefPtr ret = NULL;
-char *outbuf = NULL;
+g_auto(virSysinfoDefPtr) ret = NULL;
+g_autofree char *outbuf = NULL;
/* Some ARM systems have DMI tables available. */
if ((ret = virSysinfoReadDMI())) {
if (!virSysinfoDefIsEmpty(ret))
-return ret;
+return g_steal_pointer();
virSysinfoDefFree(ret);
}
@@ -447,29 +441,23 @@ virSysinfoReadARM(void)
virResetLastError();
if (VIR_ALLOC(ret) < 0)
-goto no_memory;
+return NULL;
if (virFileReadAll(CPUINFO, CPUINFO_FILE_LEN, ) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to open %s"), CPUINFO);
-goto no_memory;
+return NULL;
}
ret->nprocessor = 0;
ret->processor = NULL;
if (virSysinfoParseARMProcessor(outbuf, ret) < 0)
-goto no_memory;
+return NULL;
if (virSysinfoParseARMSystem(outbuf, >system) < 0)
-goto no_memory;
-
-VIR_FREE(outbuf);
-return ret;
+return NULL;
- no_memory:
-VIR_FREE(outbuf);
-virSysinfoDefFree(ret);
-return NULL;
+return g_steal_pointer();
}
static char *
@@ -606,21 +594,21 @@ virSysinfoParseS390Processor(const char *base,
virSysinfoDefPtr ret)
virSysinfoDefPtr
virSysinfoReadS390(void)
{
-virSysinfoDefPtr ret = NULL;
-char *outbuf = NULL;
+g_auto(virSysinfoDefPtr) ret = NULL;
+g_autofree char *outbuf = NULL;
if (VIR_ALLOC(ret) < 0)
-goto no_memory;
+return NULL;
/* Gather info from /proc/cpuinfo */
if (virFileReadAll(CPUINFO, CPUINFO_FILE_LEN, ) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to open %s"), CPUINFO);
-goto no_memory;
+return NULL;
}
if (virSysinfoParseS390Processor(outbuf, ret) < 0)
-goto no_memory;
+return NULL;
/* Free buffer before reading next file */
VIR_FREE(outbuf);
@@ -629,19 +617,13 @@ virSysinfoReadS390(void)
if (virFileReadAll(SYSINFO, 8192, ) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to open %s"), SYSINFO);
-goto no_memory;
+return NULL;
}
if (virSysinfoParseS390System(outbuf, >system) < 0)
-goto no_memory;
-
-VIR_FREE(outbuf);
-return ret;
+return NULL;
- no_memory:
-virSysinfoDefFree(ret);
-VIR_FREE(outbuf);
-return NULL;
+return g_steal_pointer();
}
diff --git a/src/util/virsysinfo.h b/src/util/virsysinfo.h
index f30809294b..3ce936205c 100644
--- a/src/util/virsysinfo.h
+++ b/src/util/virsysinfo.h
@@ -144,6 +144,8 @@ void virSysinfoChassisDefFree(virSysinfoChassisDefPtr def);
void virSysinfoOEMStringsDefFree(virSysinfoOEMStringsDefPtr def);
voi
[libvirt] [PATCH 2/5] openvz: Get rid of "no_memory" labels
As pointed out by Ján Tomko, "no_memory seems suspicious in the times of
abort()".
As libvirt decided to take the path to not report OOM and simply abort
when it happens, let's get rid of the no_memory labels and simplify the
code around them.
Signed-off-by: Fabiano Fidêncio
---
src/openvz/openvz_conf.c | 18 +++---
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 971adaf71c..c4c6dec2f7 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -148,18 +148,18 @@ openvzParseBarrierLimit(const char* value,
virCapsPtr openvzCapsInit(void)
{
-virCapsPtr caps;
+g_autoptr(virCaps) caps = NULL;
virCapsGuestPtr guest;
if ((caps = virCapabilitiesNew(virArchFromHost(),
false, false)) == NULL)
-goto no_memory;
+return NULL;
if (!(caps->host.numa = virCapabilitiesHostNUMANewHost()))
-goto no_memory;
+return NULL;
if (virCapabilitiesInitCaches(caps) < 0)
-goto no_memory;
+return NULL;
if ((guest = virCapabilitiesAddGuest(caps,
VIR_DOMAIN_OSTYPE_EXE,
@@ -168,7 +168,7 @@ virCapsPtr openvzCapsInit(void)
NULL,
0,
NULL)) == NULL)
-goto no_memory;
+return NULL;
if (virCapabilitiesAddGuestDomain(guest,
VIR_DOMAIN_VIRT_OPENVZ,
@@ -176,13 +176,9 @@ virCapsPtr openvzCapsInit(void)
NULL,
0,
NULL) == NULL)
-goto no_memory;
+return NULL;
-return caps;
-
- no_memory:
-virObjectUnref(caps);
-return NULL;
+return g_steal_pointer();
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] vbox: Get rid of "no_memory" labels
As pointed out by Ján Tomko, "no_memory seems suspicious in the times of
abort()".
As libvirt decided to take the path to not report OOM and simply abort
when it happens, let's get rid of the no_memory labels and simplify the
code around them.
Signed-off-by: Fabiano Fidêncio
---
src/vbox/vbox_common.c | 20
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 4493fe8582..fc67b716da 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -69,18 +69,18 @@ static virDomainDefParserConfig vboxDomainDefParserConfig =
{
static virCapsPtr
vboxCapsInit(void)
{
-virCapsPtr caps;
-virCapsGuestPtr guest;
+g_autoptr(virCaps) caps = NULL;
+virCapsGuestPtr guest = NULL;
if ((caps = virCapabilitiesNew(virArchFromHost(),
false, false)) == NULL)
-goto no_memory;
+return NULL;
if (!(caps->host.numa = virCapabilitiesHostNUMANewHost()))
-goto no_memory;
+return NULL;
if (virCapabilitiesInitCaches(caps) < 0)
-goto no_memory;
+return NULL;
if ((guest = virCapabilitiesAddGuest(caps,
VIR_DOMAIN_OSTYPE_HVM,
@@ -89,7 +89,7 @@ vboxCapsInit(void)
NULL,
0,
NULL)) == NULL)
-goto no_memory;
+return NULL;
if (virCapabilitiesAddGuestDomain(guest,
VIR_DOMAIN_VIRT_VBOX,
@@ -97,13 +97,9 @@ vboxCapsInit(void)
NULL,
0,
NULL) == NULL)
-goto no_memory;
-
-return caps;
+return NULL;
- no_memory:
-virObjectUnref(caps);
-return NULL;
+return g_steal_pointer();
}
static void
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] Get rid of "no_memory" labels
As pointed out by Ján Tomko, "no_memory seems suspicious in the times of abort()". As libvirt decided to take the path to not report OOM and simply abort when it happens, let's get rid of the no_memory labels and simplify the code around them. The two exceptions are: - phyp code, as libvirt may end up dropping this code entirely; - virfirewall.c code, as it seems we heavily really on firewall->err being set to ENOMEM; If one thinks that virfirewall.c should also be converted, please, shout out and I'll work on that. Fabiano Fidêncio (5): conf: Get rid of "no_memory" labels openvz: Get rid of "no_memory" labels rpc: Get rid of "no_memory" labels util: Get rid of "no_memory" labels vbox: Get rid of "no_memory" labels src/conf/capabilities.c | 16 ++ src/conf/domain_audit.c | 40 +++-- src/openvz/openvz_conf.c | 18 +-- src/rpc/virnetclient.c | 42 -- src/util/virsysinfo.c| 64 +++- src/util/virsysinfo.h| 2 ++ src/util/viruri.c| 28 +++--- src/vbox/vbox_common.c | 20 + 8 files changed, 75 insertions(+), 155 deletions(-) -- 2.24.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] conf: Get rid of "no_memory" labels
As pointed out by Ján Tomko, "no_memory seems suspicious in the times of
abort()".
As libvirt decided to take the path to not report OOM and simply abort
when it happens, let's get rid of the no_memory labels and simplify the
code around them.
Signed-off-by: Fabiano Fidêncio
---
src/conf/capabilities.c | 16 +++-
src/conf/domain_audit.c | 40 ++--
2 files changed, 13 insertions(+), 43 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index da54591c11..82129feaac 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -619,26 +619,16 @@
virCapabilitiesHostSecModelAddBaseLabel(virCapsHostSecModelPtr secmodel,
const char *type,
const char *label)
{
-char *t = NULL, *l = NULL;
-
if (type == NULL || label == NULL)
return -1;
-t = g_strdup(type);
-l = g_strdup(label);
-
if (VIR_EXPAND_N(secmodel->labels, secmodel->nlabels, 1) < 0)
-goto no_memory;
+return -1;
-secmodel->labels[secmodel->nlabels - 1].type = t;
-secmodel->labels[secmodel->nlabels - 1].label = l;
+secmodel->labels[secmodel->nlabels - 1].type = g_strdup(type);
+secmodel->labels[secmodel->nlabels - 1].label = g_strdup(label);
return 0;
-
- no_memory:
-VIR_FREE(l);
-VIR_FREE(t);
-return -1;
}
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index a55dcd5f91..fdccc585fb 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -89,12 +89,12 @@ virDomainAuditGenericDev(virDomainObjPtr vm,
const char *reason,
bool success)
{
-char *newdev = NULL;
-char *olddev = NULL;
+g_autofree char *newdev = NULL;
+g_autofree char *olddev = NULL;
+g_autofree char *vmname = NULL;
+g_autofree char *oldsrc = NULL;
+g_autofree char *newsrc = NULL;
char uuidstr[VIR_UUID_STRING_BUFLEN];
-char *vmname = NULL;
-char *oldsrc = NULL;
-char *newsrc = NULL;
const char *virt = virDomainAuditGetVirtType(vm->def);
/* if both new and old source aren't provided don't log anything */
@@ -107,29 +107,17 @@ virDomainAuditGenericDev(virDomainObjPtr vm,
virUUIDFormat(vm->def->uuid, uuidstr);
if (!(vmname = virAuditEncode("vm", vm->def->name)))
-goto no_memory;
+return;
if (!(newsrc = virAuditEncode(newdev, VIR_AUDIT_STR(newsrcpath
-goto no_memory;
+return;
if (!(oldsrc = virAuditEncode(olddev, VIR_AUDIT_STR(oldsrcpath
-goto no_memory;
+return;
VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
"virt=%s resrc=%s reason=%s %s uuid=%s %s %s",
virt, type, reason, vmname, uuidstr, oldsrc, newsrc);
-
- cleanup:
-VIR_FREE(newdev);
-VIR_FREE(olddev);
-VIR_FREE(vmname);
-VIR_FREE(oldsrc);
-VIR_FREE(newsrc);
-return;
-
- no_memory:
-VIR_WARN("OOM while encoding audit message");
-goto cleanup;
}
@@ -957,13 +945,13 @@ virDomainAuditInput(virDomainObjPtr vm,
bool success)
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
-char *vmname;
+g_autofree char *vmname = NULL;
const char *virt = virDomainAuditGetVirtType(vm->def);
virUUIDFormat(vm->def->uuid, uuidstr);
if (!(vmname = virAuditEncode("vm", vm->def->name)))
-goto no_memory;
+return;
switch ((virDomainInputType) input->type) {
case VIR_DOMAIN_INPUT_TYPE_MOUSE:
@@ -980,12 +968,4 @@ virDomainAuditInput(virDomainObjPtr vm,
case VIR_DOMAIN_INPUT_TYPE_LAST:
break;
}
-
- cleanup:
-VIR_FREE(vmname);
-return;
-
- no_memory:
-VIR_WARN("OOM while encoding audit message");
-goto cleanup;
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] schemas: Allow additional qemu cmd line arguments/env variables and qemuCaps to be interleaved
On Fri, Dec 20, 2019 at 10:29 AM Michal Privoznik wrote: > > While command line arguments are sort of positional (becasue you becasue -> because > have to have two entries, one for "-arg" the other for "value"), > it doesn't really matter whether env variables come before or > after command line arguments. > > And it matter even less when playing with qemu capabilities. matter -> matters > > Signed-off-by: Michal Privoznik Patch makes sense and doesn't break backward compat. I'd take this review with a grain of salt ... Reviewed-by: Fabiano Fidêncio -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 41/42] admin: Use g_autofree on getSocketPath()
On Fri, Dec 20, 2019 at 9:14 AM Michal Prívozník wrote:
>
> On 12/19/19 8:33 PM, Ján Tomko wrote:
> > On Thu, Dec 19, 2019 at 05:22:32PM +0100, Fabiano Fidêncio wrote:
> >> On Thu, Dec 19, 2019 at 5:14 PM Pavel Hrdina wrote:
> >>>
> >>> On Thu, Dec 19, 2019 at 11:04:46AM +0100, Fabiano Fidêncio wrote:
> >>> > Signed-off-by: Fabiano Fidêncio
> >>> > ---
> >>> > src/admin/libvirt-admin.c | 3 +--
> >>> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >>>
> >>> s/on/in/ $SUBJECT?
> >>>
> >>> This might be the case for other patches as well.
> >>
> >> Noted.
> >>
> >>>
> >>> One note, I would say it's ok to squash some of the patches from this
> >>> series together, for example all the g_autofree patches per file for
> >>> example.
> >>
> >> I really thought about that. However, it may be misleading as I'm not
> >> touching all the possible conversions to use g_autofree in the other
> >> functions.
> >
> > Well this case is also misleading, since you aren't touching all the
> > possible g_autofree conversions in this functions
>
>
> ACK if you squash this in:
>
> diff --git i/src/admin/libvirt-admin.c w/src/admin/libvirt-admin.c
> index dcbb8ca84d..4099a54854 100644
> --- i/src/admin/libvirt-admin.c
> +++ w/src/admin/libvirt-admin.c
> @@ -100,11 +100,11 @@ static char *
> getSocketPath(virURIPtr uri)
> {
> g_autofree char *rundir = virGetUserRuntimeDirectory();
> -char *sock_path = NULL;
> +g_autofree char *sock_path = NULL;
> size_t i = 0;
>
> if (!uri)
> -goto cleanup;
> +return NULL;
>
>
> for (i = 0; i < uri->paramsCount; i++) {
> @@ -116,7 +116,7 @@ getSocketPath(virURIPtr uri)
> } else {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("Unknown URI parameter '%s'"), param->name);
> -goto error;
> +return NULL;
> }
> }
>
> @@ -127,7 +127,7 @@ getSocketPath(virURIPtr uri)
> if (!uri->scheme) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> "%s", _("No URI scheme specified"));
> -goto error;
> +return NULL;
> }
> if (STREQ(uri->scheme, "libvirtd")) {
> legacy = true;
> @@ -135,7 +135,7 @@ getSocketPath(virURIPtr uri)
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("Unsupported URI scheme '%s'"),
> uri->scheme);
> -goto error;
> +return NULL;
> }
>
> if (legacy) {
> @@ -152,16 +152,11 @@ getSocketPath(virURIPtr uri)
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("Invalid URI path '%s', try '/system'"),
> NULLSTR_EMPTY(uri->path));
> -goto error;
> +return NULL;
> }
> }
>
> - cleanup:
> -return sock_path;
> -
> - error:
> -VIR_FREE(sock_path);
> -goto cleanup;
> +return g_steal_pointer(_path);
> }
>
>
> Michal
Thanks, Michal.
I really haven't thought about using g_steal_pointer in this case.
Taking a "review 101", I'd really expect this to be pointed out as:
- If I missed this, it happened most likely because either I'm not
aware of this or I didn't see the opportunity of doing such change;
- Saying something could be changed but not pointing out what makes
the life of **any** developer harder as the developer has to solve a
puzzle in order to understand the comment;
Please, for the next round, consider (and, at this point, you already
know that) that I'm not as smart as the reviewer and point out what
has to be changed. :-)
Michal went one step further and even provided the diff, it's not
needed, but I'd really expect to be pointed to what's wrong during the
review.
I'll go ahead and push the series.
Best Regards,
--
Fabiano Fidêncio
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/42] rpc: Use g_autofree on virNetClientNewLibSSH2()
On Fri, Dec 20, 2019 at 12:30 AM Daniel Henrique Barboza
wrote:
>
>
>
> On 12/19/19 7:17 PM, Fabiano Fidêncio wrote:
> > On Thu, Dec 19, 2019 at 8:00 PM Daniel Henrique Barboza
> > wrote:
> >>
> >>
> >>
> >> On 12/19/19 7:04 AM, Fabiano Fidêncio wrote:
> >>> Signed-off-by: Fabiano Fidêncio
> >>> ---
> >>>src/rpc/virnetclient.c | 18 ++
> >>>1 file changed, 6 insertions(+), 12 deletions(-)
> >>>
> >> [...]
> >>
> >>>if (knownHostsPath) {
> >>> @@ -517,12 +517,6 @@ virNetClientPtr virNetClientNewLibSSH2(const char
> >>> *host,
> >>>goto cleanup;
> >>>
> >>> cleanup:
> >>> -VIR_FREE(command);
> >>> -VIR_FREE(privkey);
> >>> -VIR_FREE(knownhosts);
> >>> -VIR_FREE(homedir);
> >>> -VIR_FREE(confdir);
> >>> -VIR_FREE(nc);
> >>>return ret;
> >>>
> >>
> >> This will leave a now unneeded 'cleanup' label:
> >>
> >>cleanup:
> >> return ret;
> >>
> >>
> >> In a quick glance at the patch series I noticed that Patch 41 also leaves
> >> an unused 'cleanup' label as well after the changes.
> >
> > Daniel,
> >
> > Please, take a look at the no_memory label. It calls the cleanup one.
>
> I am afraid I wasn't clear. By 'unneeded' I mean a label that is a simply
> 'return'
> call. 'cleanup' is doing no cleanups anymore. So this this guy:
>
> no_memory:
> virReportOOMError();
> goto cleanup;
>
> turns into:
>
> no_memory:
> virReportOOMError();
> return ret;
>
>
> And in fact, since ret is initialized with NULL, and it will never be set
> before
> a no_memory jump, you can even do:
>
> no_memory:
> virReportOOMError();
> return NULL;
>
> All 'cleanup' jumps can be changed to 'return NULL' and the line that sets
> 'ret'
> can turn into
>
> return virNetClientNew(sock, NULL));
>
> And you can get rid of both the 'cleanup' label and the 'ret' variable.
>
>
> All of this is just me being pedantic though :) I saw that you erased some
> 'exit' labels in other patches and wanted to point out that this label is
> also removable. In the end I believe you can keep this patch just adding
> g_autofree and removing VIR_FREE calls and, if you want, remove these labels
> in a separated patch.
I see. I'll approach this in a different series, tho.
Best Regards,
--
Fabiano Fidêncio
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 16/42] rpc: Don't check the output of virGetUserConfigDirectory()
On Thu, Dec 19, 2019 at 8:23 PM Ján Tomko wrote:
>
> On Thu, Dec 19, 2019 at 11:04:21AM +0100, Fabiano Fidêncio wrote:
> >virGetUserConfigDirectory() *never* *ever* returns NULL, making the
> >checks for it completely unnecessary.
> >
> >Signed-off-by: Fabiano Fidêncio
> >---
> > src/rpc/virnetclient.c | 11 ---
> > 1 file changed, 4 insertions(+), 7 deletions(-)
> >
> >diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
> >index 40e5fa35e2..eba8b865d1 100644
> >--- a/src/rpc/virnetclient.c
> >+++ b/src/rpc/virnetclient.c
> >@@ -455,11 +455,9 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
> > knownhosts = g_strdup(knownHostsPath);
> > } else {
> > confdir = virGetUserConfigDirectory();
> >-if (confdir) {
> >-virBufferAsprintf(, "%s/known_hosts", confdir);
> >-if (!(knownhosts = virBufferContentAndReset()))
> >-goto no_memory;
> >-}
> >+virBufferAsprintf(, "%s/known_hosts", confdir);
> >+if (!(knownhosts = virBufferContentAndReset()))
> >+goto no_memory;
>
> no_memory seems suspicious in the times of abort()
Indeed. But that's material for another series.
Best Regards,
--
Fabiano Fidêncio
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/42] rpc: Use g_autofree on virNetClientNewLibSSH2()
On Thu, Dec 19, 2019 at 8:00 PM Daniel Henrique Barboza
wrote:
>
>
>
> On 12/19/19 7:04 AM, Fabiano Fidêncio wrote:
> > Signed-off-by: Fabiano Fidêncio
> > ---
> > src/rpc/virnetclient.c | 18 ++
> > 1 file changed, 6 insertions(+), 12 deletions(-)
> >
> [...]
>
> > if (knownHostsPath) {
> > @@ -517,12 +517,6 @@ virNetClientPtr virNetClientNewLibSSH2(const char
> > *host,
> > goto cleanup;
> >
> >cleanup:
> > -VIR_FREE(command);
> > -VIR_FREE(privkey);
> > -VIR_FREE(knownhosts);
> > -VIR_FREE(homedir);
> > -VIR_FREE(confdir);
> > -VIR_FREE(nc);
> > return ret;
> >
>
> This will leave a now unneeded 'cleanup' label:
>
> cleanup:
> return ret;
>
>
> In a quick glance at the patch series I noticed that Patch 41 also leaves
> an unused 'cleanup' label as well after the changes.
Daniel,
Please, take a look at the no_memory label. It calls the cleanup one.
Best Regards,
--
Fabiano Fidêncio
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 41/42] admin: Use g_autofree on getSocketPath()
On Thu, Dec 19, 2019 at 8:32 PM Ján Tomko wrote: > > On Thu, Dec 19, 2019 at 05:22:32PM +0100, Fabiano Fidêncio wrote: > >On Thu, Dec 19, 2019 at 5:14 PM Pavel Hrdina wrote: > >> > >> On Thu, Dec 19, 2019 at 11:04:46AM +0100, Fabiano Fidêncio wrote: > >> > Signed-off-by: Fabiano Fidêncio > >> > --- > >> > src/admin/libvirt-admin.c | 3 +-- > >> > 1 file changed, 1 insertion(+), 2 deletions(-) > >> > >> s/on/in/ $SUBJECT? > >> > >> This might be the case for other patches as well. > > > >Noted. > > > >> > >> One note, I would say it's ok to squash some of the patches from this > >> series together, for example all the g_autofree patches per file for > >> example. > > > >I really thought about that. However, it may be misleading as I'm not > >touching all the possible conversions to use g_autofree in the other > >functions. > > Well this case is also misleading, since you aren't touching all the > possible g_autofree conversions in this functions If you're talking specifically about this patch, sock_path is returned. Meaning that we cannot free it when its out of the scope. If you caught some other case, please let me know because as I most likely missed it. Best Regards, -- Fabiano Fidêncio -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/42] storage: Don't check the output of virGetUserConfigDirectory()
On Thu, Dec 19, 2019 at 5:51 PM Ján Tomko wrote:
>
> On Thu, Dec 19, 2019 at 11:04:13AM +0100, Fabiano Fidêncio wrote:
> >virGetUserConfigDirectory() *never* *ever* returns NULL, making the
> >checks for it completely unnecessary.
> >
> >Signed-off-by: Fabiano Fidêncio
> >---
> > src/storage/storage_driver.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> >index 580a5e6f15..71078dfbd6 100644
> >--- a/src/storage/storage_driver.c
> >+++ b/src/storage/storage_driver.c
> >@@ -278,7 +278,7 @@ storageStateInitialize(bool privileged,
> > } else {
> > configdir = virGetUserConfigDirectory();
> > rundir = virGetUserRuntimeDirectory();
> >-if (!(configdir && rundir))
> >+if (!rundir)
> > goto error;
> >
>
> Looking at virGetUserRuntimeDirectory, that one should always return as
> well, so you can delete them both (adjusting the commit message)
It's done later in the series, when I change all the
virGetUserRuntimeDirectory() cases.
Best Regards,
--
Fabiano Fidêncio
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 41/42] admin: Use g_autofree on getSocketPath()
On Thu, Dec 19, 2019 at 5:14 PM Pavel Hrdina wrote: > > On Thu, Dec 19, 2019 at 11:04:46AM +0100, Fabiano Fidêncio wrote: > > Signed-off-by: Fabiano Fidêncio > > --- > > src/admin/libvirt-admin.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > s/on/in/ $SUBJECT? > > This might be the case for other patches as well. Noted. > > One note, I would say it's ok to squash some of the patches from this > series together, for example all the g_autofree patches per file for > example. I really thought about that. However, it may be misleading as I'm not touching all the possible conversions to use g_autofree in the other functions. > > Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 05/42] rpc: Don't check the output of virGetUserDirectory()
On Thu, Dec 19, 2019 at 5:07 PM Pavel Hrdina wrote:
>
> On Thu, Dec 19, 2019 at 11:04:10AM +0100, Fabiano Fidêncio wrote:
> > virGetUserDirectory() *never* *ever* returns NULL, making the checks for
> > it completely unnecessary.
> >
> > Signed-off-by: Fabiano Fidêncio
> > ---
> > src/rpc/virnetclient.c | 12
> > src/rpc/virnettlscontext.c | 12
> > 2 files changed, 4 insertions(+), 20 deletions(-)
>
> [...]
>
> > diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
> > index ec9dd35c46..08944f6771 100644
> > --- a/src/rpc/virnettlscontext.c
> > +++ b/src/rpc/virnettlscontext.c
> > @@ -805,9 +805,6 @@ static int virNetTLSContextLocateCredentials(const char
> > *pkipath,
> > */
> > userdir = virGetUserDirectory();
> >
> > -if (!userdir)
> > -goto error;
> > -
> > user_pki_path = g_strdup_printf("%s/.pki/libvirt", userdir);
> >
> > VIR_DEBUG("Trying to find TLS user credentials in %s",
> > user_pki_path);
> > @@ -864,15 +861,6 @@ static int virNetTLSContextLocateCredentials(const
> > char *pkipath,
> > VIR_FREE(userdir);
> >
> > return 0;
> > -
> > - error:
> > -VIR_FREE(*cacert);
> > -VIR_FREE(*cacrl);
> > -VIR_FREE(*key);
> > -VIR_FREE(*cert);
> > -VIR_FREE(user_pki_path);
> > -VIR_FREE(userdir);
> > -return -1;
>
> This doesn't look right. Looks like some leftover from rebase where
> you wanted to use g_autofree.
No, actually not. The only usage of `goto error` was when checking the
result of virGetUserDirectory().
By removing the check, we have also to remove the label.
Best Regards,
--
Fabiano Fidêncio
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] AUTHORS: Add Fabiano Fidêncio
On Thu, Dec 19, 2019 at 4:31 PM Ján Tomko wrote: > > $ git log --committer=fidencio --pretty=oneline | wc -l > 12 > > Signed-off-by: Ján Tomko > --- > Déjà vu from c379576dbc80a66820e256f9ce27595270d95ac2 except the > mailmap entry is already set up. > > AUTHORS.in | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/AUTHORS.in b/AUTHORS.in > index fbac54c48d..4ed8ceb858 100644 > --- a/AUTHORS.in > +++ b/AUTHORS.in > @@ -19,6 +19,7 @@ Daniel Veillard > Doug Goldstein > Eric Blake > Erik Skultety > +Fabiano Fidêncio > Gao Feng > Guido Günther > Ján Tomko > -- > 2.21.0 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list Reviewed-by: Fabiano Fidêncio ? :-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 36/42] locking: Use g_autofree on virLockManagerLockDaemonPath()
Signed-off-by: Fabiano Fidêncio
---
src/locking/lock_driver_lockd.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index e8f0329b05..8ca77e525d 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -122,14 +122,12 @@ static char *virLockManagerLockDaemonPath(bool privileged)
if (privileged) {
path = g_strdup(RUNSTATEDIR "/libvirt/virtlockd-sock");
} else {
-char *rundir = NULL;
+g_autofree char *rundir = NULL;
if (!(rundir = virGetUserRuntimeDirectory()))
return NULL;
path = g_strdup_printf("%s/virtlockd-sock", rundir);
-
-VIR_FREE(rundir);
}
return path;
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 33/42] logging: Use g_autofree on virLogDaemonUnixSocketPaths()
Together with the change, let's also simplify the function and get rid
of the goto.
Signed-off-by: Fabiano Fidêncio
---
src/logging/log_daemon.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
index 268d3c62b9..d41d9caba9 100644
--- a/src/logging/log_daemon.c
+++ b/src/logging/log_daemon.c
@@ -391,29 +391,23 @@ virLogDaemonUnixSocketPaths(bool privileged,
*sockfile = g_strdup(RUNSTATEDIR "/libvirt/virtlogd-sock");
*adminSockfile = g_strdup(RUNSTATEDIR "/libvirt/virtlogd-admin-sock");
} else {
-char *rundir = NULL;
+g_autofree char *rundir = NULL;
mode_t old_umask;
if (!(rundir = virGetUserRuntimeDirectory()))
-goto error;
+return -1;
old_umask = umask(077);
if (virFileMakePath(rundir) < 0) {
umask(old_umask);
-VIR_FREE(rundir);
-goto error;
+return -1;
}
umask(old_umask);
*sockfile = g_strdup_printf("%s/virtlogd-sock", rundir);
*adminSockfile = g_strdup_printf("%s/virtlogd-admin-sock", rundir);
-
-VIR_FREE(rundir);
}
return 0;
-
- error:
-return -1;
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 37/42] locking: Use g_autofree on virLockDaemonUnixSocketPaths()
Together with the change, let's also simplify the function and get rid
of the goto.
Signed-off-by: Fabiano Fidêncio
---
src/locking/lock_daemon.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 0d12a97231..9bcd36a869 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -449,29 +449,23 @@ virLockDaemonUnixSocketPaths(bool privileged,
*sockfile = g_strdup(RUNSTATEDIR "/libvirt/virtlockd-sock");
*adminSockfile = g_strdup(RUNSTATEDIR "/libvirt/virtlockd-admin-sock");
} else {
-char *rundir = NULL;
+g_autofree char *rundir = NULL;
mode_t old_umask;
if (!(rundir = virGetUserRuntimeDirectory()))
-goto error;
+return -1;
old_umask = umask(077);
if (virFileMakePath(rundir) < 0) {
-VIR_FREE(rundir);
umask(old_umask);
-goto error;
+return -1;
}
umask(old_umask);
*sockfile = g_strdup_printf("%s/virtlockd-sock", rundir);
*adminSockfile = g_strdup_printf("%s/virtlockd-admin-sock", rundir);
-
-VIR_FREE(rundir);
}
return 0;
-
- error:
-return -1;
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 41/42] admin: Use g_autofree on getSocketPath()
Signed-off-by: Fabiano Fidêncio
---
src/admin/libvirt-admin.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index f156736d9f..d0c191a56a 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -99,7 +99,7 @@ virAdmInitialize(void)
static char *
getSocketPath(virURIPtr uri)
{
-char *rundir = virGetUserRuntimeDirectory();
+g_autofree char *rundir = virGetUserRuntimeDirectory();
char *sock_path = NULL;
size_t i = 0;
@@ -160,7 +160,6 @@ getSocketPath(virURIPtr uri)
}
cleanup:
-VIR_FREE(rundir);
return sock_path;
error:
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 21/42] logging: Don't check the output of virGetUserConfigDirectory()
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/logging/log_daemon_config.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/logging/log_daemon_config.c b/src/logging/log_daemon_config.c
index ab42921140..97f2de90a6 100644
--- a/src/logging/log_daemon_config.c
+++ b/src/logging/log_daemon_config.c
@@ -44,16 +44,12 @@ virLogDaemonConfigFilePath(bool privileged, char
**configfile)
} else {
g_autofree char *configdir = NULL;
-if (!(configdir = virGetUserConfigDirectory()))
-goto error;
+configdir = virGetUserConfigDirectory();
*configfile = g_strdup_printf("%s/virtlogd.conf", configdir);
}
return 0;
-
- error:
-return -1;
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 29/42] qemu: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/qemu/qemu_conf.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index aa96d50e41..c07a844dfc 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -171,8 +171,6 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool
privileged)
cfg->cacheDir = g_strdup_printf("%s/qemu/cache", cachedir);
rundir = virGetUserRuntimeDirectory();
-if (!rundir)
-return NULL;
cfg->stateDir = g_strdup_printf("%s/qemu/run", rundir);
cfg->swtpmStateDir = g_strdup_printf("%s/swtpm", cfg->stateDir);
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 19/42] network: Don't check the output of virGetUserConfigDirectory()
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/network/bridge_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index e360645969..98aa530715 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -741,7 +741,7 @@ networkStateInitialize(bool privileged,
} else {
configdir = virGetUserConfigDirectory();
rundir = virGetUserRuntimeDirectory();
-if (!(configdir && rundir))
+if (!rundir)
goto error;
network_driver->networkConfigDir = g_strdup_printf("%s/qemu/networks",
configdir);
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 25/42] storage: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/storage/storage_driver.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 71078dfbd6..a33328db37 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -278,8 +278,6 @@ storageStateInitialize(bool privileged,
} else {
configdir = virGetUserConfigDirectory();
rundir = virGetUserRuntimeDirectory();
-if (!rundir)
-goto error;
driver->configDir = g_strdup_printf("%s/storage", configdir);
driver->autostartDir = g_strdup_printf("%s/storage/autostart",
configdir);
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 40/42] interface: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/interface/interface_backend_netcf.c | 3 +--
src/interface/interface_backend_udev.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/interface/interface_backend_netcf.c
b/src/interface/interface_backend_netcf.c
index 4f46717cf3..65cb7eae62 100644
--- a/src/interface/interface_backend_netcf.c
+++ b/src/interface/interface_backend_netcf.c
@@ -105,8 +105,7 @@ netcfStateInitialize(bool privileged,
} else {
g_autofree char *rundir = NULL;
-if (!(rundir = virGetUserRuntimeDirectory()))
-goto error;
+rundir = virGetUserRuntimeDirectory();
driver->stateDir = g_strdup_printf("%s/interface/run", rundir);
}
diff --git a/src/interface/interface_backend_udev.c
b/src/interface/interface_backend_udev.c
index 26b1045f8a..7cc098eb33 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -1160,8 +1160,7 @@ udevStateInitialize(bool privileged,
} else {
g_autofree char *rundir = NULL;
-if (!(rundir = virGetUserRuntimeDirectory()))
-goto cleanup;
+rundir = virGetUserRuntimeDirectory();
driver->stateDir = g_strdup_printf("%s/interface/run", rundir);
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 35/42] logging: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/logging/log_daemon.c | 11 +++
src/logging/log_manager.c | 3 +--
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
index 3fee2b3b57..488f3b459d 100644
--- a/src/logging/log_daemon.c
+++ b/src/logging/log_daemon.c
@@ -394,8 +394,7 @@ virLogDaemonUnixSocketPaths(bool privileged,
g_autofree char *rundir = NULL;
mode_t old_umask;
-if (!(rundir = virGetUserRuntimeDirectory()))
-return -1;
+rundir = virGetUserRuntimeDirectory();
old_umask = umask(077);
if (virFileMakePath(rundir) < 0) {
@@ -615,8 +614,7 @@ virLogDaemonExecRestartStatePath(bool privileged,
g_autofree char *rundir = NULL;
mode_t old_umask;
-if (!(rundir = virGetUserRuntimeDirectory()))
-return -1;
+rundir = virGetUserRuntimeDirectory();
old_umask = umask(077);
if (virFileMakePath(rundir) < 0) {
@@ -992,10 +990,7 @@ int main(int argc, char **argv) {
if (privileged) {
run_dir = g_strdup(RUNSTATEDIR "/libvirt");
} else {
-if (!(run_dir = virGetUserRuntimeDirectory())) {
-VIR_ERROR(_("Can't determine user directory"));
-goto cleanup;
-}
+run_dir = virGetUserRuntimeDirectory();
}
if (privileged)
diff --git a/src/logging/log_manager.c b/src/logging/log_manager.c
index 7a4f036240..6b66218116 100644
--- a/src/logging/log_manager.c
+++ b/src/logging/log_manager.c
@@ -49,8 +49,7 @@ virLogManagerDaemonPath(bool privileged)
} else {
g_autofree char *rundir = NULL;
-if (!(rundir = virGetUserRuntimeDirectory()))
-return NULL;
+rundir = virGetUserRuntimeDirectory();
path = g_strdup_printf("%s/virtlogd-sock", rundir);
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 20/42] logging: Use g_autofree on virLogDaemonConfigFilePath()
Signed-off-by: Fabiano Fidêncio
---
src/logging/log_daemon_config.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/logging/log_daemon_config.c b/src/logging/log_daemon_config.c
index 0cf9729e7f..ab42921140 100644
--- a/src/logging/log_daemon_config.c
+++ b/src/logging/log_daemon_config.c
@@ -42,13 +42,12 @@ virLogDaemonConfigFilePath(bool privileged, char
**configfile)
if (privileged) {
*configfile = g_strdup(SYSCONFDIR "/libvirt/virtlogd.conf");
} else {
-char *configdir = NULL;
+g_autofree char *configdir = NULL;
if (!(configdir = virGetUserConfigDirectory()))
goto error;
*configfile = g_strdup_printf("%s/virtlogd.conf", configdir);
-VIR_FREE(configdir);
}
return 0;
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 22/42] locking: Use g_autofree on virLockDaemonConfigFilePath()
Signed-off-by: Fabiano Fidêncio
---
src/locking/lock_daemon_config.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/locking/lock_daemon_config.c b/src/locking/lock_daemon_config.c
index d7e13013d7..62df30d9f4 100644
--- a/src/locking/lock_daemon_config.c
+++ b/src/locking/lock_daemon_config.c
@@ -41,13 +41,12 @@ virLockDaemonConfigFilePath(bool privileged, char
**configfile)
if (privileged) {
*configfile = g_strdup(SYSCONFDIR "/libvirt/virtlockd.conf");
} else {
-char *configdir = NULL;
+g_autofree char *configdir = NULL;
if (!(configdir = virGetUserConfigDirectory()))
goto error;
*configfile = g_strdup_printf("%s/virtlockd.conf", configdir);
-VIR_FREE(configdir);
}
return 0;
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 32/42] logging: Use g_autofree on virLogManagerDaemonPath()
Signed-off-by: Fabiano Fidêncio
---
src/logging/log_manager.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/logging/log_manager.c b/src/logging/log_manager.c
index e191093272..7a4f036240 100644
--- a/src/logging/log_manager.c
+++ b/src/logging/log_manager.c
@@ -47,14 +47,12 @@ virLogManagerDaemonPath(bool privileged)
if (privileged) {
path = g_strdup(RUNSTATEDIR "/libvirt/virtlogd-sock");
} else {
-char *rundir = NULL;
+g_autofree char *rundir = NULL;
if (!(rundir = virGetUserRuntimeDirectory()))
return NULL;
path = g_strdup_printf("%s/virtlogd-sock", rundir);
-
-VIR_FREE(rundir);
}
return path;
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 16/42] rpc: Don't check the output of virGetUserConfigDirectory()
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/rpc/virnetclient.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 40e5fa35e2..eba8b865d1 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -455,11 +455,9 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
knownhosts = g_strdup(knownHostsPath);
} else {
confdir = virGetUserConfigDirectory();
-if (confdir) {
-virBufferAsprintf(, "%s/known_hosts", confdir);
-if (!(knownhosts = virBufferContentAndReset()))
-goto no_memory;
-}
+virBufferAsprintf(, "%s/known_hosts", confdir);
+if (!(knownhosts = virBufferContentAndReset()))
+goto no_memory;
}
if (privkeyPath) {
@@ -556,8 +554,7 @@ virNetClientPtr virNetClientNewLibssh(const char *host,
knownhosts = g_strdup(knownHostsPath);
} else {
confdir = virGetUserConfigDirectory();
-if (confdir)
-knownhosts = g_strdup_printf("%s/known_hosts", confdir);
+knownhosts = g_strdup_printf("%s/known_hosts", confdir);
}
if (privkeyPath) {
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 30/42] node_device: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/node_device/node_device_hal.c | 3 +--
src/node_device/node_device_udev.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/node_device/node_device_hal.c
b/src/node_device/node_device_hal.c
index b40f93df46..cf12854fe9 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -618,8 +618,7 @@ nodeStateInitialize(bool privileged G_GNUC_UNUSED,
} else {
g_autofree char *rundir = NULL;
-if (!(rundir = virGetUserRuntimeDirectory()))
-goto failure;
+rundir = virGetUserRuntimeDirectory();
driver->stateDir = g_strdup_printf("%s/nodedev/run", rundir);
}
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index cae00dc9dc..eedcd123a3 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1806,8 +1806,7 @@ nodeStateInitialize(bool privileged,
} else {
g_autofree char *rundir = NULL;
-if (!(rundir = virGetUserRuntimeDirectory()))
-goto cleanup;
+rundir = virGetUserRuntimeDirectory();
driver->stateDir = g_strdup_printf("%s/nodedev/run", rundir);
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 34/42] logging: Use g_autofree on virLogDaemonExecRestartStatePath()
Together with the change, let's also simplify the function and get rid
of the goto.
Signed-off-by: Fabiano Fidêncio
---
src/logging/log_daemon.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
index d41d9caba9..3fee2b3b57 100644
--- a/src/logging/log_daemon.c
+++ b/src/logging/log_daemon.c
@@ -612,29 +612,23 @@ virLogDaemonExecRestartStatePath(bool privileged,
if (privileged) {
*state_file = g_strdup(RUNSTATEDIR "/virtlogd-restart-exec.json");
} else {
-char *rundir = NULL;
+g_autofree char *rundir = NULL;
mode_t old_umask;
if (!(rundir = virGetUserRuntimeDirectory()))
-goto error;
+return -1;
old_umask = umask(077);
if (virFileMakePath(rundir) < 0) {
umask(old_umask);
-VIR_FREE(rundir);
-goto error;
+return -1;
}
umask(old_umask);
*state_file = g_strdup_printf("%s/virtlogd-restart-exec.json", rundir);
-
-VIR_FREE(rundir);
}
return 0;
-
- error:
-return -1;
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/42] secret: Don't check the output of virGetUserConfigDirectory()
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/secret/secret_driver.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index 93b4256450..cdc4b7dcf9 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -475,8 +475,7 @@ secretStateInitialize(bool privileged,
g_autofree char *rundir = NULL;
g_autofree char *cfgdir = NULL;
-if (!(cfgdir = virGetUserConfigDirectory()))
-goto error;
+cfgdir = virGetUserConfigDirectory();
driver->configDir = g_strdup_printf("%s/secrets/", cfgdir);
if (!(rundir = virGetUserRuntimeDirectory()))
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 42/42] admin: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/admin/libvirt-admin.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index d0c191a56a..dcbb8ca84d 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -147,9 +147,6 @@ getSocketPath(virURIPtr uri)
if (STREQ_NULLABLE(uri->path, "/system")) {
sock_path = g_strdup_printf(RUNSTATEDIR "/libvirt/%s", sockbase);
} else if (STREQ_NULLABLE(uri->path, "/session")) {
-if (!rundir)
-goto error;
-
sock_path = g_strdup_printf("%s/%s", rundir, sockbase);
} else {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 27/42] rpc: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/rpc/virnetsocket.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index b98287e6d7..f072afe857 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -684,8 +684,7 @@ int virNetSocketNewConnectUNIX(const char *path,
goto cleanup;
}
-if (!(rundir = virGetUserRuntimeDirectory()))
-goto cleanup;
+rundir = virGetUserRuntimeDirectory();
if (virFileMakePathWithMode(rundir, 0700) < 0) {
virReportSystemError(errno,
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 39/42] locking: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/locking/lock_daemon.c | 11 +++
src/locking/lock_driver_lockd.c | 3 +--
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 7c89adf077..65c38139c4 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -452,8 +452,7 @@ virLockDaemonUnixSocketPaths(bool privileged,
g_autofree char *rundir = NULL;
mode_t old_umask;
-if (!(rundir = virGetUserRuntimeDirectory()))
-return -1;
+rundir = virGetUserRuntimeDirectory();
old_umask = umask(077);
if (virFileMakePath(rundir) < 0) {
@@ -823,8 +822,7 @@ virLockDaemonExecRestartStatePath(bool privileged,
g_autofree char *rundir = NULL;
mode_t old_umask;
-if (!(rundir = virGetUserRuntimeDirectory()))
-return -1;
+rundir = virGetUserRuntimeDirectory();
old_umask = umask(077);
if (virFileMakePath(rundir) < 0) {
@@ -1224,10 +1222,7 @@ int main(int argc, char **argv) {
if (privileged) {
run_dir = g_strdup(RUNSTATEDIR "/libvirt");
} else {
-if (!(run_dir = virGetUserRuntimeDirectory())) {
-VIR_ERROR(_("Can't determine user directory"));
-goto cleanup;
-}
+run_dir = virGetUserRuntimeDirectory();
}
if (privileged)
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index 8ca77e525d..339e2f6949 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -124,8 +124,7 @@ static char *virLockManagerLockDaemonPath(bool privileged)
} else {
g_autofree char *rundir = NULL;
-if (!(rundir = virGetUserRuntimeDirectory()))
-return NULL;
+rundir = virGetUserRuntimeDirectory();
path = g_strdup_printf("%s/virtlockd-sock", rundir);
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 31/42] network: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/network/bridge_driver.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 98aa530715..c9c45df758 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -741,8 +741,6 @@ networkStateInitialize(bool privileged,
} else {
configdir = virGetUserConfigDirectory();
rundir = virGetUserRuntimeDirectory();
-if (!rundir)
-goto error;
network_driver->networkConfigDir = g_strdup_printf("%s/qemu/networks",
configdir);
network_driver->networkAutostartDir =
g_strdup_printf("%s/qemu/networks/autostart", configdir);
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 38/42] locking: Use g_autofree on virLockDaemonExecRestartStatePath()
Together with the change, let's also simplify the function and get rid
of the goto.
Signed-off-by: Fabiano Fidêncio
---
src/locking/lock_daemon.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 9bcd36a869..7c89adf077 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -820,29 +820,23 @@ virLockDaemonExecRestartStatePath(bool privileged,
if (privileged) {
*state_file = g_strdup(RUNSTATEDIR "/virtlockd-restart-exec.json");
} else {
-char *rundir = NULL;
+g_autofree char *rundir = NULL;
mode_t old_umask;
if (!(rundir = virGetUserRuntimeDirectory()))
-goto error;
+return -1;
old_umask = umask(077);
if (virFileMakePath(rundir) < 0) {
umask(old_umask);
-VIR_FREE(rundir);
-goto error;
+return -1;
}
umask(old_umask);
*state_file = g_strdup_printf("%s/virtlockd-restart-exec.json",
rundir);
-
-VIR_FREE(rundir);
}
return 0;
-
- error:
-return -1;
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 15/42] qemu: Don't check the output of virGetUserCacheDirectory()
virGetUserCacheDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/qemu/qemu_conf.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 389ac55bee..634c65095e 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -165,8 +165,6 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool
privileged)
g_autofree char *cachedir = NULL;
cachedir = virGetUserCacheDirectory();
-if (!cachedir)
-return NULL;
cfg->logDir = g_strdup_printf("%s/qemu/log", cachedir);
cfg->swtpmLogDir = g_strdup_printf("%s/qemu/log", cachedir);
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 24/42] util: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/util/virhostdev.c | 3 +--
src/util/virpidfile.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 78e409732a..9b4ea30216 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -182,8 +182,7 @@ virHostdevManagerNew(void)
g_autofree char *rundir = NULL;
mode_t old_umask;
-if (!(rundir = virGetUserRuntimeDirectory()))
-return NULL;
+rundir = virGetUserRuntimeDirectory();
hostdevMgr->stateDir = g_strdup_printf("%s/hostdevmgr", rundir);
diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c
index 249515aff2..b08e0d8d52 100644
--- a/src/util/virpidfile.c
+++ b/src/util/virpidfile.c
@@ -488,8 +488,7 @@ virPidFileConstructPath(bool privileged,
}
*pidfile = g_strdup_printf("%s/%s.pid", runstatedir, progname);
} else {
-if (!(rundir = virGetUserRuntimeDirectory()))
-return -1;
+rundir = virGetUserRuntimeDirectory();
if (virFileMakePathWithMode(rundir, 0700) < 0) {
virReportSystemError(errno,
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 28/42] remote: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/remote/remote_daemon.c | 8 +---
src/remote/remote_driver.c | 3 +--
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index b400b1dd10..5893492875 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -245,8 +245,7 @@ daemonUnixSocketPaths(struct daemonConfig *config,
} else {
mode_t old_umask;
-if (!(rundir = virGetUserRuntimeDirectory()))
-goto cleanup;
+rundir = virGetUserRuntimeDirectory();
old_umask = umask(077);
if (virFileMakePath(rundir) < 0) {
@@ -1208,11 +1207,6 @@ int main(int argc, char **argv) {
run_dir = g_strdup(RUNSTATEDIR "/libvirt");
} else {
run_dir = virGetUserRuntimeDirectory();
-
-if (!run_dir) {
-VIR_ERROR(_("Can't determine user directory"));
-goto cleanup;
-}
}
if (privileged)
old_umask = umask(022);
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index e684fcba09..c11f73ab4d 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -779,8 +779,7 @@ remoteGetUNIXSocketHelper(remoteDriverTransport transport,
remoteDriverTransportTypeToString(transport));
return NULL;
}
-if (!(userdir = virGetUserRuntimeDirectory()))
-return NULL;
+userdir = virGetUserRuntimeDirectory();
sockname = g_strdup_printf("%s/%s-sock", userdir, sock_prefix);
} else {
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/42] storage: Don't check the output of virGetUserConfigDirectory()
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/storage/storage_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 580a5e6f15..71078dfbd6 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -278,7 +278,7 @@ storageStateInitialize(bool privileged,
} else {
configdir = virGetUserConfigDirectory();
rundir = virGetUserRuntimeDirectory();
-if (!(configdir && rundir))
+if (!rundir)
goto error;
driver->configDir = g_strdup_printf("%s/storage", configdir);
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 14/42] util: Don't check the output of virGetUserCacheDirectory()
virGetUserCacheDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/util/virlog.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/util/virlog.c b/src/util/virlog.c
index 10639d7328..6bae56e2e3 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -180,8 +180,7 @@ virLogSetDefaultOutputToFile(const char *binary, bool
privileged)
virLogDefaultOutput = g_strdup_printf("%d:file:%s/log/libvirt/%s.log",
virLogDefaultPriority,
LOCALSTATEDIR, binary);
} else {
-if (!(logdir = virGetUserCacheDirectory()))
-return -1;
+logdir = virGetUserCacheDirectory();
old_umask = umask(077);
if (virFileMakePath(logdir) < 0) {
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 12/42] vbox: Don't check the output of virGetUserCacheDirectory()
virGetUserCacheDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/vbox/vbox_common.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 586937fa19..4493fe8582 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -7374,9 +7374,8 @@ vboxDomainScreenshot(virDomainPtr dom,
if (privileged) {
cacheDir = g_strdup_printf("%s/cache/libvirt", LOCALSTATEDIR);
-} else if (!(cacheDir = virGetUserCacheDirectory())) {
-VBOX_RELEASE(machine);
-return NULL;
+} else {
+cacheDir = virGetUserCacheDirectory();
}
tmp = g_strdup_printf("%s/vbox.screendump.XX", cacheDir);
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 17/42] remote: Don't check the output of virGetUserConfigDirectory()
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/remote/remote_daemon_config.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/remote/remote_daemon_config.c
b/src/remote/remote_daemon_config.c
index ae6b491686..84e331474b 100644
--- a/src/remote/remote_daemon_config.c
+++ b/src/remote/remote_daemon_config.c
@@ -81,17 +81,13 @@ daemonConfigFilePath(bool privileged, char **configfile)
} else {
char *configdir = NULL;
-if (!(configdir = virGetUserConfigDirectory()))
-goto error;
+configdir = virGetUserConfigDirectory();
*configfile = g_strdup_printf("%s/%s.conf", configdir, DAEMON_NAME);
VIR_FREE(configdir);
}
return 0;
-
- error:
-return -1;
}
struct daemonConfig*
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/42] tools: Use g_autofree on cmdCd()
Signed-off-by: Fabiano Fidêncio
---
tools/vsh.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index 5ccda5ab44..bbb6227130 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -3175,8 +3175,7 @@ bool
cmdCd(vshControl *ctl, const vshCmd *cmd)
{
const char *dir = NULL;
-char *dir_malloced = NULL;
-bool ret = true;
+g_autofree char *dir_malloced = NULL;
char ebuf[1024];
if (!ctl->imode) {
@@ -3192,11 +3191,10 @@ cmdCd(vshControl *ctl, const vshCmd *cmd)
if (chdir(dir) == -1) {
vshError(ctl, _("cd: %s: %s"),
virStrerror(errno, ebuf, sizeof(ebuf)), dir);
-ret = false;
+return false;
}
-VIR_FREE(dir_malloced);
-return ret;
+return true;
}
const vshCmdOptDef opts_echo[] = {
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 07/42] util: Don't check the output of virGetUserConfigDirectory()
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/util/virauth.c | 3 +--
src/util/virconf.c | 2 --
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/util/virauth.c b/src/util/virauth.c
index 55208c01ef..f75e674586 100644
--- a/src/util/virauth.c
+++ b/src/util/virauth.c
@@ -66,8 +66,7 @@ virAuthGetConfigFilePathURI(virURIPtr uri,
}
}
-if (!(userdir = virGetUserConfigDirectory()))
-return -1;
+userdir = virGetUserConfigDirectory();
*path = g_strdup_printf("%s/auth.conf", userdir);
diff --git a/src/util/virconf.c b/src/util/virconf.c
index dce84cabb7..cd18ea61e6 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -1507,8 +1507,6 @@ virConfLoadConfigPath(const char *name)
path = g_strdup_printf("%s/libvirt/%s", SYSCONFDIR, name);
} else {
char *userdir = virGetUserConfigDirectory();
-if (!userdir)
-return NULL;
path = g_strdup_printf("%s/%s", userdir, name);
VIR_FREE(userdir);
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 18/42] qemu: Don't check the output of virGetUserConfigDirectory()
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/qemu/qemu_conf.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 634c65095e..aa96d50e41 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -177,8 +177,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool
privileged)
cfg->swtpmStateDir = g_strdup_printf("%s/swtpm", cfg->stateDir);
-if (!(cfg->configBaseDir = virGetUserConfigDirectory()))
-return NULL;
+cfg->configBaseDir = virGetUserConfigDirectory();
cfg->libDir = g_strdup_printf("%s/qemu/lib", cfg->configBaseDir);
cfg->saveDir = g_strdup_printf("%s/qemu/save", cfg->configBaseDir);
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 26/42] secret: Don't check the output of virGetUserRuntimeDirectory()
virGetUserRuntimeDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/secret/secret_driver.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index cdc4b7dcf9..096672f114 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -478,8 +478,7 @@ secretStateInitialize(bool privileged,
cfgdir = virGetUserConfigDirectory();
driver->configDir = g_strdup_printf("%s/secrets/", cfgdir);
-if (!(rundir = virGetUserRuntimeDirectory()))
-goto error;
+rundir = virGetUserRuntimeDirectory();
driver->stateDir = g_strdup_printf("%s/secrets/run", rundir);
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/42] qemu: Don't check the output of virGetUserDirectory()
virGetUserDirectory() *never* *ever* returns NULL, making the checks for
it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/qemu/qemu_interop_config.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/qemu/qemu_interop_config.c b/src/qemu/qemu_interop_config.c
index cbabde45df..f5f419e630 100644
--- a/src/qemu/qemu_interop_config.c
+++ b/src/qemu/qemu_interop_config.c
@@ -118,9 +118,6 @@ qemuInteropFetchConfigs(const char *name,
if (!xdgConfig) {
g_autofree char *home = virGetUserDirectory();
-if (!home)
-return -1;
-
xdgConfig = g_strdup_printf("%s/.config", home);
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/42] rpc: Don't check the output of virGetUserDirectory()
virGetUserDirectory() *never* *ever* returns NULL, making the checks for
it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/rpc/virnetclient.c | 12
src/rpc/virnettlscontext.c | 12
2 files changed, 4 insertions(+), 20 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 1b882a261a..40e5fa35e2 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -466,10 +466,8 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
privkey = g_strdup(privkeyPath);
} else {
homedir = virGetUserDirectory();
-if (homedir) {
-if (virNetClientFindDefaultSshKey(homedir, ) < 0)
-goto no_memory;
-}
+if (virNetClientFindDefaultSshKey(homedir, ) < 0)
+goto no_memory;
}
if (!authMethods) {
@@ -566,10 +564,8 @@ virNetClientPtr virNetClientNewLibssh(const char *host,
privkey = g_strdup(privkeyPath);
} else {
homedir = virGetUserDirectory();
-if (homedir) {
-if (virNetClientFindDefaultSshKey(homedir, ) < 0)
-goto no_memory;
-}
+if (virNetClientFindDefaultSshKey(homedir, ) < 0)
+goto no_memory;
}
if (!authMethods) {
diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index ec9dd35c46..08944f6771 100644
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -805,9 +805,6 @@ static int virNetTLSContextLocateCredentials(const char
*pkipath,
*/
userdir = virGetUserDirectory();
-if (!userdir)
-goto error;
-
user_pki_path = g_strdup_printf("%s/.pki/libvirt", userdir);
VIR_DEBUG("Trying to find TLS user credentials in %s", user_pki_path);
@@ -864,15 +861,6 @@ static int virNetTLSContextLocateCredentials(const char
*pkipath,
VIR_FREE(userdir);
return 0;
-
- error:
-VIR_FREE(*cacert);
-VIR_FREE(*cacrl);
-VIR_FREE(*key);
-VIR_FREE(*cert);
-VIR_FREE(user_pki_path);
-VIR_FREE(userdir);
-return -1;
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/42] vbox: Use g_autofree on vboxDomainScreenshot()
This also fixes a cacheDir's leak when g_mkstep_full() fails.
Signed-off-by: Fabiano Fidêncio
---
src/vbox/vbox_common.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 5957ae52c4..586937fa19 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -7341,8 +7341,8 @@ vboxDomainScreenshot(virDomainPtr dom,
vboxIID iid;
IMachine *machine = NULL;
nsresult rc;
-char *tmp;
-char *cacheDir;
+g_autofree char *tmp = NULL;
+g_autofree char *cacheDir = NULL;
int tmp_fd = -1;
unsigned int max_screen;
bool privileged = geteuid() == 0;
@@ -7383,7 +7383,6 @@ vboxDomainScreenshot(virDomainPtr dom,
if ((tmp_fd = g_mkstemp_full(tmp, O_RDWR | O_CLOEXEC, S_IRUSR | S_IWUSR))
== -1) {
virReportSystemError(errno, _("g_mkstemp(\"%s\") failed"), tmp);
-VIR_FREE(tmp);
VBOX_RELEASE(machine);
return NULL;
}
@@ -7454,8 +7453,6 @@ vboxDomainScreenshot(virDomainPtr dom,
VIR_FORCE_CLOSE(tmp_fd);
unlink(tmp);
-VIR_FREE(tmp);
-VIR_FREE(cacheDir);
VBOX_RELEASE(machine);
vboxIIDUnalloc();
return ret;
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 00/42] Cleanups after adopting g_get_*_dir()
After adopting g_get_*_dir(), internally, when implementing virGetUser*Dir(), the libvirt functions changed their behaviour as NULL is never ever returned by the GLib functions. Knowing that, let's cleanup the callers' code of those functions, removing the unnecessary checks. While doing the cleanup mentioned above, I've noticed that some other cleanups could be done, when touching the very same function, as using g_autofree. It's done as part of this series as well. phyp driver was not touched as there's some plan to just drop its code entirely: https://www.redhat.com/archives/libvir-list/2019-December/msg01162.html Fabiano Fidêncio (42): tools: Use g_autofree on cmdCd() vbox: Don't leak virGetUserDirectory()'s output rpc: Use g_autofree on virNetClientNewLibSSH2() rpc: Use g_autofree on virNetClientNewLibssh() rpc: Don't check the output of virGetUserDirectory() qemu: Don't check the output of virGetUserDirectory() util: Don't check the output of virGetUserConfigDirectory() storage: Don't check the output of virGetUserConfigDirectory() secret: Don't check the output of virGetUserConfigDirectory() tools: Don't check the output of virGetUserCacheDirectory() vbox: Use g_autofree on vboxDomainScreenshot() vbox: Don't check the output of virGetUserCacheDirectory() util: Use g_autofree on virLogSetDefaultOutputToFile() util: Don't check the output of virGetUserCacheDirectory() qemu: Don't check the output of virGetUserCacheDirectory() rpc: Don't check the output of virGetUserConfigDirectory() remote: Don't check the output of virGetUserConfigDirectory() qemu: Don't check the output of virGetUserConfigDirectory() network: Don't check the output of virGetUserConfigDirectory() logging: Use g_autofree on virLogDaemonConfigFilePath() logging: Don't check the output of virGetUserConfigDirectory() locking: Use g_autofree on virLockDaemonConfigFilePath() locking: Don't check the output of virGetUserConfigDirectory() util: Don't check the output of virGetUserRuntimeDirectory() storage: Don't check the output of virGetUserRuntimeDirectory() secret: Don't check the output of virGetUserRuntimeDirectory() rpc: Don't check the output of virGetUserRuntimeDirectory() remote: Don't check the output of virGetUserRuntimeDirectory() qemu: Don't check the output of virGetUserRuntimeDirectory() node_device: Don't check the output of virGetUserRuntimeDirectory() network: Don't check the output of virGetUserRuntimeDirectory() logging: Use g_autofree on virLogManagerDaemonPath() logging: Use g_autofree on virLogDaemonUnixSocketPaths() logging: Use g_autofree on virLogDaemonExecRestartStatePath() logging: Don't check the output of virGetUserRuntimeDirectory() locking: Use g_autofree on virLockManagerLockDaemonPath() locking: Use g_autofree on virLockDaemonUnixSocketPaths() locking: Use g_autofree on virLockDaemonExecRestartStatePath() locking: Don't check the output of virGetUserRuntimeDirectory() interface: Don't check the output of virGetUserRuntimeDirectory() admin: Use g_autofree on getSocketPath() admin: Don't check the output of virGetUserRuntimeDirectory() src/admin/libvirt-admin.c | 6 +-- src/interface/interface_backend_netcf.c | 3 +- src/interface/interface_backend_udev.c | 3 +- src/locking/lock_daemon.c | 31 +++-- src/locking/lock_daemon_config.c| 9 +--- src/locking/lock_driver_lockd.c | 7 +-- src/logging/log_daemon.c| 31 +++-- src/logging/log_daemon_config.c | 9 +--- src/logging/log_manager.c | 7 +-- src/network/bridge_driver.c | 2 - src/node_device/node_device_hal.c | 3 +- src/node_device/node_device_udev.c | 3 +- src/qemu/qemu_conf.c| 7 +-- src/qemu/qemu_interop_config.c | 3 -- src/remote/remote_daemon.c | 8 +--- src/remote/remote_daemon_config.c | 6 +-- src/remote/remote_driver.c | 3 +- src/rpc/virnetclient.c | 59 + src/rpc/virnetsocket.c | 3 +- src/rpc/virnettlscontext.c | 12 - src/secret/secret_driver.c | 6 +-- src/storage/storage_driver.c| 2 - src/util/virauth.c | 3 +- src/util/virconf.c | 2 - src/util/virhostdev.c | 3 +- src/util/virlog.c | 13 ++ src/util/virpidfile.c | 3 +- src/vbox/vbox_common.c | 12 ++--- src/vbox/vbox_storage.c | 7 ++- tools/vsh.c | 13 ++ 30 files changed, 73 insertions(+), 206 deletions(-) -- 2.24.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/42] rpc: Use g_autofree on virNetClientNewLibssh()
Signed-off-by: Fabiano Fidêncio
---
src/rpc/virnetclient.c | 18 ++
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 75e653fec8..1b882a261a 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -545,13 +545,13 @@ virNetClientPtr virNetClientNewLibssh(const char *host,
virNetClientPtr ret = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
-char *nc = NULL;
-char *command = NULL;
+g_autofree char *nc = NULL;
+g_autofree char *command = NULL;
-char *homedir = NULL;
-char *confdir = NULL;
-char *knownhosts = NULL;
-char *privkey = NULL;
+g_autofree char *homedir = NULL;
+g_autofree char *confdir = NULL;
+g_autofree char *knownhosts = NULL;
+g_autofree char *privkey = NULL;
/* Use default paths for known hosts an public keys if not provided */
if (knownHostsPath) {
@@ -609,12 +609,6 @@ virNetClientPtr virNetClientNewLibssh(const char *host,
goto cleanup;
cleanup:
-VIR_FREE(command);
-VIR_FREE(privkey);
-VIR_FREE(knownhosts);
-VIR_FREE(homedir);
-VIR_FREE(confdir);
-VIR_FREE(nc);
return ret;
no_memory:
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/42] vbox: Don't leak virGetUserDirectory()'s output
On vboxStorageVolCreateXML(), virGetUserDirectory() was called without
freeing its content later on.
Signed-off-by: Fabiano Fidêncio
---
src/vbox/vbox_storage.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/vbox/vbox_storage.c b/src/vbox/vbox_storage.c
index 790b0e4997..7f240c5333 100644
--- a/src/vbox/vbox_storage.c
+++ b/src/vbox/vbox_storage.c
@@ -410,6 +410,7 @@ vboxStorageVolCreateXML(virStoragePoolPtr pool,
resultCodeUnion resultCode;
virStorageVolPtr ret = NULL;
g_autoptr(virStorageVolDef) def = NULL;
+g_autofree char *homedir = NULL;
if (!data->vboxObj)
return ret;
@@ -442,8 +443,10 @@ vboxStorageVolCreateXML(virStoragePoolPtr pool,
}
/* If target.path isn't given, use default path ~/.VirtualBox/image_name */
-if (!def->target.path)
-def->target.path = g_strdup_printf("%s/.VirtualBox/%s",
virGetUserDirectory(), def->name);
+if (!def->target.path) {
+homedir = virGetUserDirectory();
+def->target.path = g_strdup_printf("%s/.VirtualBox/%s", homedir,
def->name);
+}
VBOX_UTF8_TO_UTF16(def->target.path, );
if (!hddFormatUtf16 || !hddNameUtf16)
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/42] tools: Don't check the output of virGetUserCacheDirectory()
virGetUserCacheDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
tools/vsh.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index bbb6227130..b982aeb359 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2913,11 +2913,6 @@ vshReadlineInit(vshControl *ctl)
*/
userdir = virGetUserCacheDirectory();
-if (userdir == NULL) {
-vshError(ctl, "%s", _("Could not determine home directory"));
-goto cleanup;
-}
-
ctl->historydir = g_strdup_printf("%s/%s", userdir, ctl->name);
ctl->historyfile = g_strdup_printf("%s/history", ctl->historydir);
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 13/42] util: Use g_autofree on virLogSetDefaultOutputToFile()
Signed-off-by: Fabiano Fidêncio
---
src/util/virlog.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/src/util/virlog.c b/src/util/virlog.c
index d45e2dd316..10639d7328 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -173,8 +173,7 @@ virLogSetDefaultOutputToJournald(void)
static int
virLogSetDefaultOutputToFile(const char *binary, bool privileged)
{
-int ret = -1;
-char *logdir = NULL;
+g_autofree char *logdir = NULL;
mode_t old_umask;
if (privileged) {
@@ -182,12 +181,12 @@ virLogSetDefaultOutputToFile(const char *binary, bool
privileged)
virLogDefaultPriority,
LOCALSTATEDIR, binary);
} else {
if (!(logdir = virGetUserCacheDirectory()))
-goto cleanup;
+return -1;
old_umask = umask(077);
if (virFileMakePath(logdir) < 0) {
umask(old_umask);
-goto cleanup;
+return -1;
}
umask(old_umask);
@@ -195,10 +194,7 @@ virLogSetDefaultOutputToFile(const char *binary, bool
privileged)
virLogDefaultPriority, logdir,
binary);
}
-ret = 0;
- cleanup:
-VIR_FREE(logdir);
-return ret;
+return 0;
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 23/42] locking: Don't check the output of virGetUserConfigDirectory()
virGetUserConfigDirectory() *never* *ever* returns NULL, making the
checks for it completely unnecessary.
Signed-off-by: Fabiano Fidêncio
---
src/locking/lock_daemon_config.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/locking/lock_daemon_config.c b/src/locking/lock_daemon_config.c
index 62df30d9f4..c67b0a75e1 100644
--- a/src/locking/lock_daemon_config.c
+++ b/src/locking/lock_daemon_config.c
@@ -43,16 +43,12 @@ virLockDaemonConfigFilePath(bool privileged, char
**configfile)
} else {
g_autofree char *configdir = NULL;
-if (!(configdir = virGetUserConfigDirectory()))
-goto error;
+configdir = virGetUserConfigDirectory();
*configfile = g_strdup_printf("%s/virtlockd.conf", configdir);
}
return 0;
-
- error:
-return -1;
}
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
