Re: [libvirt] [PATCH] cpu: add CLZERO CPUID support for AMD platforms
> On Dec 16, 2019, at 5:35 PM, Jiri Denemark wrote:
>
> On Tue, Dec 03, 2019 at 03:09:12 -0800, Ani Sinha wrote:
>> Qemu commit e900135dcfb67 ("i386: Add CPUID bit for CLZERO and XSAVEERPTR")
>> adds support for CLZERO CPUID bit.
>> This commit extends support for this CPUID bit into libvirt.
>>
>> Signed-off-by: Ani Sinha
>> ---
>> src/cpu_map/x86_EPYC-IBPB.xml | 1 +
>> src/cpu_map/x86_EPYC.xml | 1 +
>> src/cpu_map/x86_features.xml | 3 +++
>> 3 files changed, 5 insertions(+)
>>
>> diff --git a/src/cpu_map/x86_EPYC-IBPB.xml b/src/cpu_map/x86_EPYC-IBPB.xml
>> index 283697e..a70fbd8 100644
>> --- a/src/cpu_map/x86_EPYC-IBPB.xml
>> +++ b/src/cpu_map/x86_EPYC-IBPB.xml
>> @@ -14,6 +14,7 @@
>>
>>
>>
>> +
>>
>>
>>
>> diff --git a/src/cpu_map/x86_EPYC.xml b/src/cpu_map/x86_EPYC.xml
>> index f060139..6c11d82 100644
>> --- a/src/cpu_map/x86_EPYC.xml
>> +++ b/src/cpu_map/x86_EPYC.xml
>> @@ -14,6 +14,7 @@
>>
>>
>>
>> +
>>
>>
>>
>
> We don't normally change existing CPU models as it can cause issues with
> migration between non-matching versions of libvirt. If QEMU enables a
> new feature for a given model (which doesn't seem to be the case of
> clzero and EPYC, however), you'd automatically get it just by asking for
> EPYC in libvirt. Otherwise, you need to ask for clzero explicitly
> (adding it to libvirt's EPYC would not cause the feature to be enabled
> anyway). Or you can use host-model CPU in which case, clzero will be
> automatically added as long as it is supported by QEMU.
Right. Thanks for clarifying this.
>
>> diff --git a/src/cpu_map/x86_features.xml b/src/cpu_map/x86_features.xml
>> index 2bed1e0..dd62755 100644
>> --- a/src/cpu_map/x86_features.xml
>> +++ b/src/cpu_map/x86_features.xml
>> @@ -473,6 +473,9 @@
>>
>>
>>
>> +
>> +
>> +
>>
>>
>>
>
> We want to keep the bits sorted by their position, I moved it up a bit,
> just above wbnoinvd.
Cool. I will keep this in mind next time I send a patch around CPUIDs.
>
> Also several cputest results have to be updated since our CPU data
> describe some CPUs which already support clzero.
>
> That said, I dropped the changes to EPYC* CPU models, moved the feature
> definition in x86_features a bit, updated the test results, and pushed
> this as commit 1d17f881a278c408ede19c7e6a5330e3f19fe0f0
>
> Reviewed-by: Jiri Denemark
Thanks for pushing this.
Ani
--
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 03/42] rpc: Use g_autofree on virNetClientNewLibSSH2()
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.
Thanks,
DHB
Best Regards,
--
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
[libvirt] [PATCH v4 1/4] qemu_process.c: use g_autofree
Change all feasible strings and scalar pointers to use g_autofree.
Signed-off-by: Daniel Henrique Barboza
---
src/qemu/qemu_process.c | 97 +++--
1 file changed, 34 insertions(+), 63 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7e1db50e8f..d6d6a9f09b 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -105,7 +105,7 @@ qemuProcessRemoveDomainStatus(virQEMUDriverPtr driver,
virDomainObjPtr vm)
{
char ebuf[1024];
-char *file = NULL;
+g_autofree char *file = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
@@ -114,7 +114,6 @@ qemuProcessRemoveDomainStatus(virQEMUDriverPtr driver,
if (unlink(file) < 0 && errno != ENOENT && errno != ENOTDIR)
VIR_WARN("Failed to remove domain XML for %s: %s",
vm->def->name, virStrerror(errno, ebuf, sizeof(ebuf)));
-VIR_FREE(file);
if (priv->pidfile &&
unlink(priv->pidfile) < 0 &&
@@ -1501,7 +1500,7 @@ qemuProcessHandleBlockThreshold(qemuMonitorPtr mon
G_GNUC_UNUSED,
virDomainDiskDefPtr disk;
virStorageSourcePtr src;
unsigned int idx;
-char *dev = NULL;
+g_autofree char *dev = NULL;
const char *path = NULL;
virObjectLock(vm);
@@ -1514,11 +1513,9 @@ qemuProcessHandleBlockThreshold(qemuMonitorPtr mon
G_GNUC_UNUSED,
if (virStorageSourceIsLocalStorage(src))
path = src->path;
-if ((dev = qemuDomainDiskBackingStoreGetName(disk, src, idx))) {
+if ((dev = qemuDomainDiskBackingStoreGetName(disk, src, idx)))
event = virDomainEventBlockThresholdNewFromObj(vm, dev, path,
threshold, excess);
-VIR_FREE(dev);
-}
}
virObjectUnlock(vm);
@@ -2052,7 +2049,7 @@ static int
qemuProcessReportLogError(qemuDomainLogContextPtr logCtxt,
const char *msgprefix)
{
-char *logmsg = NULL;
+g_autofree char *logmsg = NULL;
size_t max;
max = VIR_ERROR_MAX_LENGTH - 1;
@@ -2069,7 +2066,6 @@ qemuProcessReportLogError(qemuDomainLogContextPtr logCtxt,
else
virReportError(VIR_ERR_INTERNAL_ERROR, _("%s: %s"), msgprefix, logmsg);
-VIR_FREE(logmsg);
return 0;
}
@@ -2089,7 +2085,7 @@ qemuProcessLookupPTYs(virDomainChrDefPtr *devices,
int count,
virHashTablePtr info)
{
-char *id = NULL;
+g_autofree char *id = NULL;
size_t i;
int ret = -1;
@@ -2098,7 +2094,7 @@ qemuProcessLookupPTYs(virDomainChrDefPtr *devices,
if (chr->source->type == VIR_DOMAIN_CHR_TYPE_PTY) {
qemuMonitorChardevInfoPtr entry;
-VIR_FREE(id);
+g_free(id);
id = g_strdup_printf("char%s", chr->info.alias);
entry = virHashLookup(info, id);
@@ -2118,14 +2114,13 @@ qemuProcessLookupPTYs(virDomainChrDefPtr *devices,
}
}
-VIR_FREE(chr->source->data.file.path);
+g_free(chr->source->data.file.path);
chr->source->data.file.path = g_strdup(entry->ptyPath);
}
}
ret = 0;
cleanup:
-VIR_FREE(id);
return ret;
}
@@ -2178,7 +2173,7 @@ qemuProcessRefreshChannelVirtioState(virQEMUDriverPtr
driver,
int agentReason = VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_CHANNEL;
qemuMonitorChardevInfoPtr entry;
virObjectEventPtr event = NULL;
-char *id = NULL;
+g_autofree char *id = NULL;
if (booted)
agentReason =
VIR_CONNECT_DOMAIN_EVENT_AGENT_LIFECYCLE_REASON_DOMAIN_STARTED;
@@ -2204,8 +2199,6 @@ qemuProcessRefreshChannelVirtioState(virQEMUDriverPtr
driver,
chr->state = entry->state;
}
}
-
-VIR_FREE(id);
}
@@ -2622,7 +2615,7 @@ qemuProcessSetupPid(virDomainObjPtr vm,
virBitmapPtr use_cpumask = NULL;
virBitmapPtr afinity_cpumask = NULL;
g_autoptr(virBitmap) hostcpumap = NULL;
-char *mem_mask = NULL;
+g_autofree char *mem_mask = NULL;
int ret = -1;
if ((period || quota) &&
@@ -2702,7 +2695,6 @@ qemuProcessSetupPid(virDomainObjPtr vm,
ret = 0;
cleanup:
-VIR_FREE(mem_mask);
if (cgroup) {
if (ret < 0)
virCgroupRemove(cgroup);
@@ -2781,7 +2773,7 @@ qemuProcessKillManagedPRDaemon(virDomainObjPtr vm)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
virErrorPtr orig_err;
-char *pidfile;
+g_autofree char *pidfile = NULL;
if (!(pidfile = qemuProcessBuildPRHelperPidfilePath(vm))) {
VIR_WARN("Unable to construct pr-helper pidfile path");
@@ -2802,8 +2794,6 @@ qemuProcessKillManagedPRDaemon(virDomainObjPtr vm)
}
}
virErrorRestore(_err);
-
-VIR_FREE(pidfile);
}
@@ -2812,7 +2802,7 @@ qemuProcessStartPRDaemonHook(void *opaque)
{
[libvirt] [PATCH v4 4/4] qemu_process.c: remove 'cleanup' label from qemuProcessCreatePretendCmd()
The 'cleanup' flag is doing no cleaup in this function. We can
remove it and return NULL on error or qemuBuildCommandLine().
Signed-off-by: Daniel Henrique Barboza
---
As mentioned in the cover, sending this one in its own
patch simply because it is not related to the cleanup
made in this series - I spotted by code inspection. The
maintainer is welcome to squash this one in the previous
patch.
src/qemu/qemu_process.c | 37 -
1 file changed, 16 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 184bd6e816..2a629c0de7 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7152,11 +7152,9 @@ qemuProcessCreatePretendCmd(virQEMUDriverPtr driver,
bool standalone,
unsigned int flags)
{
-virCommandPtr cmd = NULL;
-
-virCheckFlagsGoto(VIR_QEMU_PROCESS_START_COLD |
- VIR_QEMU_PROCESS_START_PAUSED |
- VIR_QEMU_PROCESS_START_AUTODESTROY, cleanup);
+virCheckFlags(VIR_QEMU_PROCESS_START_COLD |
+ VIR_QEMU_PROCESS_START_PAUSED |
+ VIR_QEMU_PROCESS_START_AUTODESTROY, NULL);
flags |= VIR_QEMU_PROCESS_START_PRETEND;
flags |= VIR_QEMU_PROCESS_START_NEW;
@@ -7165,26 +7163,23 @@ qemuProcessCreatePretendCmd(virQEMUDriverPtr driver,
if (qemuProcessInit(driver, vm, NULL, QEMU_ASYNC_JOB_NONE,
!!migrateURI, flags) < 0)
-goto cleanup;
+return NULL;
if (qemuProcessPrepareDomain(driver, vm, flags) < 0)
-goto cleanup;
+return NULL;
VIR_DEBUG("Building emulator command line");
-cmd = qemuBuildCommandLine(driver,
- NULL,
- driver->securityManager,
- vm,
- migrateURI,
- NULL,
- VIR_NETDEV_VPORT_PROFILE_OP_NO_OP,
- standalone,
- enableFips,
- NULL,
- NULL);
-
- cleanup:
-return cmd;
+return qemuBuildCommandLine(driver,
+NULL,
+driver->securityManager,
+vm,
+migrateURI,
+NULL,
+VIR_NETDEV_VPORT_PROFILE_OP_NO_OP,
+standalone,
+enableFips,
+NULL,
+NULL);
}
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 3/4] qemu_process.c: remove cleanup labels after g_auto*() changes
The g_auto*() changes made by the previous patches made a lot
of 'cleanup' labels obsolete. Let's remove them.
Signed-off-by: Daniel Henrique Barboza
---
src/qemu/qemu_process.c | 182
1 file changed, 70 insertions(+), 112 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 4870b23704..184bd6e816 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2071,7 +2071,6 @@ qemuProcessLookupPTYs(virDomainChrDefPtr *devices,
{
g_autofree char *id = NULL;
size_t i;
-int ret = -1;
for (i = 0; i < count; i++) {
virDomainChrDefPtr chr = devices[i];
@@ -2089,7 +2088,7 @@ qemuProcessLookupPTYs(virDomainChrDefPtr *devices,
*/
virReportError(VIR_ERR_INTERNAL_ERROR,
_("no assigned pty for device %s"), id);
-goto cleanup;
+return -1;
} else {
/* 'info chardev' had no pty path for this chardev,
* but the log output had, so we're fine
@@ -2103,9 +2102,7 @@ qemuProcessLookupPTYs(virDomainChrDefPtr *devices,
}
}
-ret = 0;
- cleanup:
-return ret;
+return 0;
}
static int
@@ -2704,7 +2701,6 @@ static int
qemuProcessResctrlCreate(virQEMUDriverPtr driver,
virDomainObjPtr vm)
{
-int ret = -1;
size_t i = 0;
g_autoptr(virCaps) caps = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
@@ -2723,7 +2719,7 @@ qemuProcessResctrlCreate(virQEMUDriverPtr driver,
if (virResctrlAllocCreate(caps->host.resctrl,
vm->def->resctrls[i]->alloc,
priv->machineName) < 0)
-goto cleanup;
+return -1;
for (j = 0; j < vm->def->resctrls[i]->nmonitors; j++) {
virDomainResctrlMonDefPtr mon = NULL;
@@ -2731,13 +2727,11 @@ qemuProcessResctrlCreate(virQEMUDriverPtr driver,
mon = vm->def->resctrls[i]->monitors[j];
if (virResctrlMonitorCreate(mon->instance,
priv->machineName) < 0)
-goto cleanup;
+return -1;
}
}
-ret = 0;
- cleanup:
-return ret;
+return 0;
}
@@ -2952,10 +2946,9 @@ qemuProcessInitPasswords(virQEMUDriverPtr driver,
}
if (ret < 0)
-goto cleanup;
+return ret;
}
- cleanup:
return ret;
}
@@ -3183,7 +3176,7 @@ qemuProcessStartCPUs(virQEMUDriverPtr driver,
virDomainObjPtr vm,
/* Bring up netdevs before starting CPUs */
if (qemuInterfaceStartDevices(vm->def) < 0)
- goto cleanup;
+ return -1;
VIR_DEBUG("Using lock state '%s'", NULLSTR(priv->lockState));
if (virDomainLockProcessResume(driver->lockManager, cfg->uri,
@@ -3192,7 +3185,7 @@ qemuProcessStartCPUs(virQEMUDriverPtr driver,
virDomainObjPtr vm,
* to make sure we have state still present if the user
* tries to resume again
*/
-goto cleanup;
+return -1;
}
VIR_FREE(priv->lockState);
@@ -3213,7 +3206,6 @@ qemuProcessStartCPUs(virQEMUDriverPtr driver,
virDomainObjPtr vm,
* lifecycle event.
*/
- cleanup:
return ret;
release:
@@ -3221,7 +3213,7 @@ qemuProcessStartCPUs(virQEMUDriverPtr driver,
virDomainObjPtr vm,
if (virDomainLockProcessPause(driver->lockManager, vm, >lockState) <
0)
VIR_WARN("Unable to release lease on %s", vm->def->name);
VIR_DEBUG("Preserving lock state '%s'", NULLSTR(priv->lockState));
-goto cleanup;
+return ret;
}
@@ -3846,7 +3838,6 @@ qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr
driver,
size_t i;
bool shouldBuildHP = false;
bool shouldBuildMB = false;
-int ret = -1;
if (build) {
shouldBuildHP = qemuProcessNeedHugepagesPath(vm->def, mem);
@@ -3858,11 +3849,11 @@ qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr
driver,
path = qemuGetDomainHugepagePath(vm->def, >hugetlbfs[i]);
if (!path)
-goto cleanup;
+return -1;
if (qemuProcessBuildDestroyMemoryPathsImpl(driver, vm,
path, build) < 0)
-goto cleanup;
+return -1;
VIR_FREE(path);
}
@@ -3870,16 +3861,14 @@ qemuProcessBuildDestroyMemoryPaths(virQEMUDriverPtr
driver,
if (!build || shouldBuildMB) {
if (qemuGetMemoryBackingDomainPath(vm->def, cfg, ) < 0)
-goto cleanup;
+return -1;
if (qemuProcessBuildDestroyMemoryPathsImpl(driver, vm,
path, build) < 0)
-goto cleanup;
+return -1;
}
-ret = 0;
- cleanup:
-return ret;
+return 0;
}
@@
[libvirt] [PATCH v4 0/4] qemu_process: Glib sponsored cleanup
This series got buried a few months ago. Let's go onward unto the 20s with no one left behind. changes from version 3 [1]: - rebased to master at commit 330b556829 - removed former patch 4. The 'g_strdup_printf' change was made along the road - new patch 4: I am sending this one in separate to patch 3 because this unneeded label was already in the code, being unrelated to the changes of this series. The maintainer is welcome to squash it into patch 3. [1] https://www.redhat.com/archives/libvir-list/2019-October/msg01080.html Daniel Henrique Barboza (4): qemu_process.c: use g_autofree qemu_process.c: use g_autoptr() qemu_process.c: remove cleanup labels after g_auto*() changes qemu_process.c: remove 'cleanup' label from qemuProcessCreatePretendCmd() src/qemu/qemu_process.c | 429 +++- 1 file changed, 157 insertions(+), 272 deletions(-) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 2/4] qemu_process.c: use g_autoptr()
Change all feasible pointers to use g_autoptr().
Signed-off-by: Daniel Henrique Barboza
---
src/qemu/qemu_process.c | 113 +---
1 file changed, 37 insertions(+), 76 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index d6d6a9f09b..4870b23704 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -107,7 +107,7 @@ qemuProcessRemoveDomainStatus(virQEMUDriverPtr driver,
char ebuf[1024];
g_autofree char *file = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
file = g_strdup_printf("%s/%s.xml", cfg->stateDir, vm->def->name);
@@ -120,8 +120,6 @@ qemuProcessRemoveDomainStatus(virQEMUDriverPtr driver,
errno != ENOENT)
VIR_WARN("Failed to remove PID file for %s: %s",
vm->def->name, virStrerror(errno, ebuf, sizeof(ebuf)));
-
-virObjectUnref(cfg);
}
@@ -401,7 +399,7 @@ qemuProcessHandleReset(qemuMonitorPtr mon G_GNUC_UNUSED,
virQEMUDriverPtr driver = opaque;
virObjectEventPtr event;
qemuDomainObjPrivatePtr priv;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
int ret = -1;
virObjectLock(vm);
@@ -438,7 +436,6 @@ qemuProcessHandleReset(qemuMonitorPtr mon G_GNUC_UNUSED,
cleanup:
virObjectUnlock(vm);
virObjectEventStateQueue(driver->domainEventState, event);
-virObjectUnref(cfg);
return ret;
}
@@ -457,7 +454,7 @@ qemuProcessFakeReboot(void *opaque)
virDomainObjPtr vm = opaque;
qemuDomainObjPrivatePtr priv = vm->privateData;
virQEMUDriverPtr driver = priv->driver;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
virDomainRunningReason reason = VIR_DOMAIN_RUNNING_BOOTED;
int ret = -1, rc;
@@ -507,7 +504,6 @@ qemuProcessFakeReboot(void *opaque)
if (ret == -1)
ignore_value(qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_FORCE));
virDomainObjEndAPI();
-virObjectUnref(cfg);
}
@@ -570,7 +566,7 @@ qemuProcessHandleShutdown(qemuMonitorPtr mon G_GNUC_UNUSED,
virQEMUDriverPtr driver = opaque;
qemuDomainObjPrivatePtr priv;
virObjectEventPtr event = NULL;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
int detail = 0;
VIR_DEBUG("vm=%p", vm);
@@ -627,7 +623,6 @@ qemuProcessHandleShutdown(qemuMonitorPtr mon G_GNUC_UNUSED,
unlock:
virObjectUnlock(vm);
virObjectEventStateQueue(driver->domainEventState, event);
-virObjectUnref(cfg);
return 0;
}
@@ -642,7 +637,7 @@ qemuProcessHandleStop(qemuMonitorPtr mon G_GNUC_UNUSED,
virObjectEventPtr event = NULL;
virDomainPausedReason reason;
virDomainEventSuspendedDetailType detail;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
qemuDomainObjPrivatePtr priv = vm->privateData;
virObjectLock(vm);
@@ -688,7 +683,6 @@ qemuProcessHandleStop(qemuMonitorPtr mon G_GNUC_UNUSED,
virObjectUnlock(vm);
virObjectEventStateQueue(driver->domainEventState, event);
-virObjectUnref(cfg);
return 0;
}
@@ -701,7 +695,7 @@ qemuProcessHandleResume(qemuMonitorPtr mon G_GNUC_UNUSED,
{
virQEMUDriverPtr driver = opaque;
virObjectEventPtr event = NULL;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
qemuDomainObjPrivatePtr priv;
virDomainRunningReason reason = VIR_DOMAIN_RUNNING_UNPAUSED;
virDomainEventResumedDetailType eventDetail;
@@ -734,7 +728,6 @@ qemuProcessHandleResume(qemuMonitorPtr mon G_GNUC_UNUSED,
virObjectUnlock(vm);
virObjectEventStateQueue(driver->domainEventState, event);
-virObjectUnref(cfg);
return 0;
}
@@ -746,7 +739,7 @@ qemuProcessHandleRTCChange(qemuMonitorPtr mon G_GNUC_UNUSED,
{
virQEMUDriverPtr driver = opaque;
virObjectEventPtr event = NULL;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
virObjectLock(vm);
@@ -778,7 +771,6 @@ qemuProcessHandleRTCChange(qemuMonitorPtr mon G_GNUC_UNUSED,
virObjectUnlock(vm);
virObjectEventStateQueue(driver->domainEventState, event);
-virObjectUnref(cfg);
return 0;
}
@@ -792,7 +784,7 @@ qemuProcessHandleWatchdog(qemuMonitorPtr mon G_GNUC_UNUSED,
virQEMUDriverPtr driver = opaque;
virObjectEventPtr watchdogEvent = NULL;
virObjectEventPtr lifecycleEvent = NULL;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+
Re: [libvirt] [PATCH 41/42] admin: Use g_autofree on getSocketPath()
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 Jano Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 42/42] admin: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:47AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 40/42] interface: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:45AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 39/42] locking: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:44AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 37/42] locking: Use g_autofree on virLockDaemonUnixSocketPaths()
On Thu, Dec 19, 2019 at 11:04:42AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 38/42] locking: Use g_autofree on virLockDaemonExecRestartStatePath()
On Thu, Dec 19, 2019 at 11:04:43AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 35/42] logging: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:40AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 36/42] locking: Use g_autofree on virLockManagerLockDaemonPath()
s/on/in/ On Thu, Dec 19, 2019 at 11:04:41AM +0100, Fabiano Fidêncio wrote: Signed-off-by: Fabiano Fidêncio --- src/locking/lock_driver_lockd.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 34/42] logging: Use g_autofree on virLogDaemonExecRestartStatePath()
s/on/in/ On Thu, Dec 19, 2019 at 11:04:39AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 33/42] logging: Use g_autofree on virLogDaemonUnixSocketPaths()
On Thu, Dec 19, 2019 at 11:04:38AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 32/42] logging: Use g_autofree on virLogManagerDaemonPath()
s/on/in/ On Thu, Dec 19, 2019 at 11:04:37AM +0100, Fabiano Fidêncio wrote: Signed-off-by: Fabiano Fidêncio --- src/logging/log_manager.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 31/42] network: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:36AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 30/42] node_device: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:35AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 29/42] qemu: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:34AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 27/42] rpc: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:32AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 28/42] remote: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:33AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 26/42] secret: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:31AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 23/42] locking: Don't check the output of virGetUserConfigDirectory()
On Thu, Dec 19, 2019 at 11:04:28AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 25/42] storage: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:30AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 24/42] util: Don't check the output of virGetUserRuntimeDirectory()
On Thu, Dec 19, 2019 at 11:04:29AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 21/42] logging: Don't check the output of virGetUserConfigDirectory()
On Thu, Dec 19, 2019 at 11:04:26AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 22/42] locking: Use g_autofree on virLockDaemonConfigFilePath()
On Thu, Dec 19, 2019 at 11:04:27AM +0100, Fabiano Fidêncio wrote: Signed-off-by: Fabiano Fidêncio --- src/locking/lock_daemon_config.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 20/42] logging: Use g_autofree on virLogDaemonConfigFilePath()
s/on/in/ On Thu, Dec 19, 2019 at 11:04:25AM +0100, Fabiano Fidêncio wrote: Signed-off-by: Fabiano Fidêncio --- src/logging/log_daemon_config.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 19/42] network: Don't check the output of virGetUserConfigDirectory()
On Thu, Dec 19, 2019 at 11:04:24AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 18/42] qemu: Don't check the output of virGetUserConfigDirectory()
On Thu, Dec 19, 2019 at 11:04:23AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 17/42] remote: Don't check the output of virGetUserConfigDirectory()
On Thu, Dec 19, 2019 at 11:04:22AM +0100, Fabiano Fidêncio wrote:
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);
Another g_autofree opportunity
}
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 16/42] rpc: Don't check the output of virGetUserConfigDirectory()
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()
}
if (privkeyPath) {
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 15/42] qemu: Don't check the output of virGetUserCacheDirectory()
On Thu, Dec 19, 2019 at 11:04:20AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 14/42] util: Don't check the output of virGetUserCacheDirectory()
On Thu, Dec 19, 2019 at 11:04:19AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 13/42] util: Use g_autofree on virLogSetDefaultOutputToFile()
s/on/in/ On Thu, Dec 19, 2019 at 11:04:18AM +0100, Fabiano Fidêncio wrote: Signed-off-by: Fabiano Fidêncio --- src/util/virlog.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 12/42] vbox: Don't check the output of virGetUserCacheDirectory()
On Thu, Dec 19, 2019 at 11:04:17AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 11/42] vbox: Use g_autofree on vboxDomainScreenshot()
On Thu, Dec 19, 2019 at 11:04:16AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/42] tools: Don't check the output of virGetUserCacheDirectory()
On Thu, Dec 19, 2019 at 11:04:15AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/42] rpc: Use g_autofree on virNetClientNewLibSSH2()
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.
Thanks,
DHB
no_memory:
--
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 09/42] secret: Don't check the output of virGetUserConfigDirectory()
On Thu, Dec 19, 2019 at 11:04:14AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/42] storage: Don't check the output of virGetUserConfigDirectory()
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)
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 05/42] rpc: Don't check the output of virGetUserDirectory()
On Thu, Dec 19, 2019 at 05:42:14PM +0100, Ján Tomko wrote:
> On Thu, Dec 19, 2019 at 05:25:59PM +0100, Pavel Hrdina wrote:
> > On Thu, Dec 19, 2019 at 05:21:21PM +0100, Fabiano Fidêncio wrote:
> > > 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.
> >
> > Label yes, the VIR_FREE no as it would leak the memory.
> >
>
> There's "return 0;" right before that label.
Oh, I see, two strings are freed and the remaining ones are
transferred to caller. Somehow I missed that.
Pavel
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/42] util: Don't check the output of virGetUserConfigDirectory()
On Thu, Dec 19, 2019 at 11:04:12AM +0100, Fabiano Fidêncio wrote:
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/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;
Missed g_autofree opportunity
path = g_strdup_printf("%s/%s", userdir, name);
VIR_FREE(userdir);
Whether you squash it in or not:
Reviewed-by: Ján Tomko
Jano
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/42] qemu: Don't check the output of virGetUserDirectory()
On Thu, Dec 19, 2019 at 11:04:11AM +0100, Fabiano Fidêncio wrote: 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 05/42] rpc: Don't check the output of virGetUserDirectory()
On Thu, Dec 19, 2019 at 05:25:59PM +0100, Pavel Hrdina wrote:
On Thu, Dec 19, 2019 at 05:21:21PM +0100, Fabiano Fidêncio wrote:
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.
Label yes, the VIR_FREE no as it would leak the memory.
There's "return 0;" right before that label.
Jano
Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: PGP signature
--
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 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(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 04/42] rpc: Use g_autofree on virNetClientNewLibssh()
s/on/in/ On Thu, Dec 19, 2019 at 11:04:09AM +0100, Fabiano Fidêncio wrote: Signed-off-by: Fabiano Fidêncio --- src/rpc/virnetclient.c | 18 ++ 1 file changed, 6 insertions(+), 12 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/42] rpc: Use g_autofree on virNetClientNewLibSSH2()
s/on/in/ On Thu, Dec 19, 2019 at 11:04:08AM +0100, Fabiano Fidêncio wrote: Signed-off-by: Fabiano Fidêncio --- src/rpc/virnetclient.c | 18 ++ 1 file changed, 6 insertions(+), 12 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 05/42] rpc: Don't check the output of virGetUserDirectory()
On Thu, Dec 19, 2019 at 05:21:21PM +0100, Fabiano Fidêncio wrote:
> 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.
Label yes, the VIR_FREE no as it would leak the memory.
Pavel
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/42] vbox: Don't leak virGetUserDirectory()'s output
On Thu, Dec 19, 2019 at 11:04:07AM +0100, Fabiano Fidêncio wrote: On vboxStorageVolCreateXML(), virGetUserDirectory() was called without s/On/In/ freeing its content later on. Signed-off-by: Fabiano Fidêncio --- src/vbox/vbox_storage.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/42] tools: Use g_autofree on cmdCd()
In the commit summary: s/on/in/ On Thu, Dec 19, 2019 at 11:04:06AM +0100, Fabiano Fidêncio wrote: Signed-off-by: Fabiano Fidêncio --- tools/vsh.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 41/42] admin: Use g_autofree on getSocketPath()
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. 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. Pavel signature.asc Description: PGP signature -- 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 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.
Pavel
signature.asc
Description: PGP signature
--
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 12/19/19 4:28 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 > Yeah, once you get commit access to one of our repos, you get commit access to all of them :-D Reviewed-by: Michal Privoznik Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] CfP VHPC20: HPC Containers-Kubernetes
CALL FOR PAPERSa 15th Workshop on Virtualization in High-Performance Cloud Computing (VHPC 20) held in conjunction with the International Supercomputing Conference - High Performance, June 21-25, 2020, Frankfurt, Germany. (Springer LNCS Proceedings) Date: June 25, 2020 Workshop URL: vhpc[dot]org Abstract registration Deadline: Jan 31st, 2020 Paper Submission Deadline: Apr 05th, 2020 Springer LNCS Call for Papers Containers and virtualization technologies constitute key enabling factors for flexible resource management in modern data centers, and particularly in cloud environments. Cloud providers need to manage complex infrastructures in a seamless fashion to support the highly dynamic and heterogeneous workloads and hosted applications customers deploy. Similarly, HPC environments have been increasingly adopting techniques that enable flexible management of vast computing and networking resources, close to marginal provisioning cost, which is unprecedented in the history of scientific and commercial computing. Most recently, Function as a Service (Faas) and Serverless computing, utilizing lightweight VMs-containers widens the spectrum of applications that can be deployed in a cloud environment, especially in an HPC context. Here, HPC-provided services can be become accessible to distributed workloads outside of large cluster environments. Various virtualization-containerization technologies contribute to the overall picture in different ways: machine virtualization, with its capability to enable consolidation of multiple underutilized servers with heterogeneous software and operating systems (OSes), and its capability to live-migrate a fully operating virtual machine (VM) with a very short downtime, enables novel and dynamic ways to manage physical servers; OS-level virtualization (i.e., containerization), with its capability to isolate multiple user-space environments and to allow for their coexistence within the same OS kernel, promises to provide many of the advantages of machine virtualization with high levels of responsiveness and performance; lastly, unikernels provide for many virtualization benefits with a minimized OS/library surface. I/O Virtualization in turn allows physical network interfaces to take traffic from multiple VMs or containers; network virtualization, with its capability to create logical network overlays that are independent of the underlying physical topology is furthermore enabling virtualization of HPC infrastructures. Publication Accepted papers will be published in a Springer LNCS proceedings volume. Topics of Interest The VHPC program committee solicits original, high-quality submissions related to virtualization across the entire software stack with a special focus on the intersection of HPC, containers-virtualization and the cloud. Major Topics: - HPC workload orchestration (Kubernetes) - Kubernetes HPC batch - HPC Container Environments Landscape - HW Heterogeneity - Container ecosystem (Docker alternatives) - Networking - Lightweight Virtualization - Unikernels / LibOS - State-of-the-art processor virtualization (RISC-V, EPI) - Containerizing HPC Stacks/Apps/Codes: Climate model containers each major topic encompassing design/architecture, management, performance management, modeling and configuration/tooling. Specifically, we invite papers that deal with the following topics: - HPC orchestration (Kubernetes) - Virtualizing Kubernetes for HPC - Deployment paradigms - Multitenancy - Serverless - Declerative data center integration - Network provisioning - Storage - OCI i.a. images - Isolation/security - HW Accelerators, including GPUs, FPGAs, AI, and others - State-of-practice/art, including transition to cloud - Frameworks, system software - Programming models, runtime systems, and APIs to facilitate cloud adoption - Edge use-cases - Application adaptation, success stories - Kubernetes Batch - Scheduling, job management - Execution paradigm - workflow - Data management - Deployment paradigm - Multi-cluster/scalability - Performance improvement - Workflow / execution paradigm - Podman: end-to-end Docker alternative container environment & use-cases - Creating, Running containers as non-root (rootless) - Running rootless containers with MPI - Container live migration - Running containers in restricted environments without setuid - Networking - Software defined networks and network virtualization - New virtualization NICs/Nitro alike ASICs for the data center? - Kubernetes SDN policy (Calico i.a.) - Kubernetes network provisioning (Flannel i.a.) - Lightweight Virtualization - Micro VMMs (Rust-VMM, Firecracker, solo5) - Xen - Nitro hypervisor (KVM) - RVirt - Cloud Hypervisor - Unikernels / LibOS - HPC
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] AUTHORS: Add Fabiano Fidêncio
$ 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
Re: [libvirt] [libvirt-tck PATCH v2] Add cases for nvram
On Wed, Dec 18, 2019 at 04:52:03PM +0800, dzh...@redhat.com wrote: > From: Dan Zheng > > This is to add the tests for below flags: > - Sys::Virt::Domain::UNDEFINE_KEEP_NVRAM > - Sys::Virt::Domain::UNDEFINE_NVRAM > > v1: https://www.redhat.com/archives/libvir-list/2019-December/msg00932.html FWIW, any notes about previous versions are best put after the '---' marker... > > Signed-off-by: Dan Zheng > --- ...just here. This ensures that when the patch is applied to git, the notes get discarded from the history. > scripts/domain/401-ovmf-nvram.t | 144 > 1 file changed, 144 insertions(+) > create mode 100644 scripts/domain/401-ovmf-nvram.t Reviewed-by: Daniel P. Berrangé and pushed to git master Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] hostcpu: add support for reporting die_id in NUMA topology
Update the host CPU code to report the die_id in the NUMA topology
capabilities. On systems with mulitple dies, this fixes the bug
where CPU cores can't be distinguished:
Notes core_id is repeated within the scope of the socket.
It now reports
Signed-off-by: Daniel P. Berrangé
---
docs/schemas/capability.rng | 3 ++
src/conf/capabilities.c | 5 ++-
src/conf/capabilities.h | 1 +
src/libvirt_linux.syms| 1 +
src/util/virhostcpu.c | 16 ++
src/util/virhostcpu.h | 1 +
.../vircaps2xmldata/vircaps-aarch64-basic.xml | 32 +--
.../vircaps2xmldata/vircaps-x86_64-basic.xml | 32 +--
.../vircaps2xmldata/vircaps-x86_64-caches.xml | 16 +-
.../vircaps-x86_64-resctrl-cdp.xml| 24 +++---
.../vircaps-x86_64-resctrl-cmt.xml| 24 +++---
.../vircaps-x86_64-resctrl-fake-feature.xml | 24 +++---
.../vircaps-x86_64-resctrl-skx-twocaches.xml | 2 +-
.../vircaps-x86_64-resctrl-skx.xml| 2 +-
.../vircaps-x86_64-resctrl.xml| 24 +++---
15 files changed, 116 insertions(+), 91 deletions(-)
diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng
index 26d313d652..165f704ef3 100644
--- a/docs/schemas/capability.rng
+++ b/docs/schemas/capability.rng
@@ -265,6 +265,9 @@
+
+
+
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index e54e46f54e..2324a7cb8a 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -885,8 +885,9 @@ virCapabilitiesHostNUMAFormat(virCapsHostNUMAPtr caps,
return -1;
virBufferAsprintf(buf,
- " socket_id='%d' core_id='%d' siblings='%s'",
+ " socket_id='%d' die_id='%d' core_id='%d'
siblings='%s'",
cell->cpus[j].socket_id,
+ cell->cpus[j].die_id,
cell->cpus[j].core_id,
siblings);
VIR_FREE(siblings);
@@ -1477,6 +1478,7 @@ virCapabilitiesFillCPUInfo(int cpu_id G_GNUC_UNUSED,
cpu->id = cpu_id;
if (virHostCPUGetSocket(cpu_id, >socket_id) < 0 ||
+virHostCPUGetDie(cpu_id, >die_id) < 0 ||
virHostCPUGetCore(cpu_id, >core_id) < 0)
return -1;
@@ -1605,6 +1607,7 @@ virCapabilitiesHostNUMAInitFake(virCapsHostNUMAPtr caps)
goto error;
if (tmp) {
cpus[cid].id = id;
+cpus[cid].die_id = 0;
cpus[cid].socket_id = s;
cpus[cid].core_id = c;
if (!(cpus[cid].siblings = virBitmapNew(ncpus)))
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index 4a49e94aa5..75f29666c9 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -88,6 +88,7 @@ struct _virCapsGuest {
struct _virCapsHostNUMACellCPU {
unsigned int id;
unsigned int socket_id;
+unsigned int die_id;
unsigned int core_id;
virBitmapPtr siblings;
};
diff --git a/src/libvirt_linux.syms b/src/libvirt_linux.syms
index 5fa2c790ef..55649ae39c 100644
--- a/src/libvirt_linux.syms
+++ b/src/libvirt_linux.syms
@@ -4,6 +4,7 @@
# util/virhostcpu.h
virHostCPUGetCore;
+virHostCPUGetDie;
virHostCPUGetInfoPopulateLinux;
virHostCPUGetSiblingsList;
virHostCPUGetSocket;
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index 22102f2c75..0c174702a4 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -218,6 +218,22 @@ virHostCPUGetSocket(unsigned int cpu, unsigned int *socket)
return 0;
}
+int
+virHostCPUGetDie(unsigned int cpu, unsigned int *die)
+{
+int ret = virFileReadValueUint(die,
+ "%s/cpu/cpu%u/topology/die_id",
+ SYSFS_SYSTEM_PATH, cpu);
+
+/* If the file is not there, it's 0 */
+if (ret == -2)
+*die = 0;
+else if (ret < 0)
+return -1;
+
+return 0;
+}
+
int
virHostCPUGetCore(unsigned int cpu, unsigned int *core)
{
diff --git a/src/util/virhostcpu.h b/src/util/virhostcpu.h
index d95d380d4a..9be2e51a38 100644
--- a/src/util/virhostcpu.h
+++ b/src/util/virhostcpu.h
@@ -65,6 +65,7 @@ int virHostCPUStatsAssign(virNodeCPUStatsPtr param,
#ifdef __linux__
int virHostCPUGetSocket(unsigned int cpu, unsigned int *socket);
+int virHostCPUGetDie(unsigned int cpu, unsigned int *die);
int virHostCPUGetCore(unsigned int cpu, unsigned int *core);
virBitmapPtr virHostCPUGetSiblingsList(unsigned int cpu);
diff --git a/tests/vircaps2xmldata/vircaps-aarch64-basic.xml
[libvirt] [PATCH 3/5] qemu: add support for specifying CPU "dies" topology parameter
QEMU since 4.1.0 supports the "dies" parameter for -smp
Signed-off-by: Daniel P. Berrangé
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 9 +++--
.../caps_4.1.0.x86_64.xml | 1 +
.../caps_4.2.0.aarch64.xml| 1 +
.../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 +
.../qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 +
.../caps_4.2.0.x86_64.xml | 1 +
.../hugepages-nvdimm.x86_64-latest.args | 2 +-
...memory-default-hugepage.x86_64-latest.args | 2 +-
.../memfd-memory-numa.x86_64-latest.args | 2 +-
...y-hotplug-nvdimm-access.x86_64-latest.args | 2 +-
...ry-hotplug-nvdimm-align.x86_64-latest.args | 2 +-
...ry-hotplug-nvdimm-label.x86_64-latest.args | 2 +-
...ory-hotplug-nvdimm-pmem.x86_64-latest.args | 2 +-
...hotplug-nvdimm-readonly.x86_64-latest.args | 2 +-
.../memory-hotplug-nvdimm.x86_64-latest.args | 2 +-
tests/qemuxml2argvdata/smp-dies.args | 29
tests/qemuxml2argvdata/smp-dies.xml | 33 +++
tests/qemuxml2argvtest.c | 1 +
20 files changed, 86 insertions(+), 12 deletions(-)
create mode 100644 tests/qemuxml2argvdata/smp-dies.args
create mode 100644 tests/qemuxml2argvdata/smp-dies.xml
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2223589058..29a0d48d4a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -553,6 +553,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
"blockdev-file-dynamic-auto-read-only",
"savevm-monitor-nodes",
"drive-nvme",
+ "smp-dies",
);
@@ -2966,6 +2967,7 @@ static struct virQEMUCapsCommandLineProps
virQEMUCapsCommandLine[] = {
{ "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST },
{ "chardev", "fd", QEMU_CAPS_CHARDEV_FD_PASS },
{ "overcommit", NULL, QEMU_CAPS_OVERCOMMIT },
+{ "smp-opts", "dies", QEMU_CAPS_SMP_DIES },
};
static int
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 1b2522126c..2f92b479bd 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -534,6 +534,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
QEMU_CAPS_BLOCK_FILE_AUTO_READONLY_DYNAMIC, /* the auto-read-only property
of block backends for files is dynamic */
QEMU_CAPS_SAVEVM_MONITOR_NODES, /* 'savevm' handles monitor-owned nodes
properly */
QEMU_CAPS_DRIVE_NVME, /* -drive file.driver=nvme */
+QEMU_CAPS_SMP_DIES, /* -smp dies= */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4066ecacd3..83a77c2348 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7080,7 +7080,8 @@ qemuBuildTSEGCommandLine(virCommandPtr cmd,
static int
qemuBuildSmpCommandLine(virCommandPtr cmd,
-virDomainDefPtr def)
+virDomainDefPtr def,
+virQEMUCapsPtr qemuCaps)
{
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
unsigned int maxvcpus = virDomainDefGetVcpusMax(def);
@@ -7105,12 +7106,14 @@ qemuBuildSmpCommandLine(virCommandPtr cmd,
/* sockets, cores, and threads are either all zero
* or all non-zero, thus checking one of them is enough */
if (def->cpu && def->cpu->sockets) {
-if (def->cpu->dies != 1) {
+if (def->cpu->dies != 1 && !virQEMUCapsGet(qemuCaps,
QEMU_CAPS_SMP_DIES)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Only 1 die per socket is supported"));
return -1;
}
virBufferAsprintf(, ",sockets=%u", def->cpu->sockets);
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SMP_DIES))
+virBufferAsprintf(, ",dies=%u", def->cpu->dies);
virBufferAsprintf(, ",cores=%u", def->cpu->cores);
virBufferAsprintf(, ",threads=%u", def->cpu->threads);
} else {
@@ -9798,7 +9801,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
if (qemuBuildMemCommandLine(cmd, cfg, def, qemuCaps, priv) < 0)
return NULL;
-if (qemuBuildSmpCommandLine(cmd, def) < 0)
+if (qemuBuildSmpCommandLine(cmd, def, qemuCaps) < 0)
return NULL;
if (qemuBuildIOThreadCommandLine(cmd, def) < 0)
diff --git a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
index 2bc9672063..12a538cb54 100644
--- a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
@@ -214,6 +214,7 @@
+
4001000
0
43100241
diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml
b/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml
index 588e682064..3c038983d4 100644
---
[libvirt] [PATCH 5/5] tests: add host CPU data files for validating die_id
Only Cascadelake-AP CPUs appear to report "die_id" values != 0 on Linux right now - AMD EPYC's don't report "die_id" (at least with Fedora 31 kernel). Lacking access to Cascadelake-AP CPUs, this test data was from a Fedora 31 QEMU guest launched with -cpu qemu64 -smp sockets=2,dies=3,cores=2,threads=1 Ideally we'd replace this data with some from a real machine reporting "die_id", to ensure we're not mislead by QEMU's impl. Signed-off-by: Daniel P. Berrangé --- .../linux-basic-dies/system/cpu | 1 + .../linux-basic-dies/system/node | 1 + .../vircaps-x86_64-basic-dies.xml | 35 ++ tests/vircaps2xmltest.c | 1 + .../cpu/cpu0/topology/core_cpus | 1 + .../cpu/cpu0/topology/core_cpus_list | 1 + .../linux-with-die/cpu/cpu0/topology/core_id | 1 + .../cpu/cpu0/topology/core_siblings | 1 + .../cpu/cpu0/topology/core_siblings_list | 1 + .../linux-with-die/cpu/cpu0/topology/die_cpus | 1 + .../cpu/cpu0/topology/die_cpus_list | 1 + .../linux-with-die/cpu/cpu0/topology/die_id | 1 + .../cpu/cpu0/topology/package_cpus| 1 + .../cpu/cpu0/topology/package_cpus_list | 1 + .../cpu/cpu0/topology/physical_package_id | 1 + .../cpu/cpu0/topology/thread_siblings | 1 + .../cpu/cpu0/topology/thread_siblings_list| 1 + .../linux-with-die/cpu/cpu1/online| 1 + .../cpu/cpu1/topology/core_cpus | 1 + .../cpu/cpu1/topology/core_cpus_list | 1 + .../linux-with-die/cpu/cpu1/topology/core_id | 1 + .../cpu/cpu1/topology/core_siblings | 1 + .../cpu/cpu1/topology/core_siblings_list | 1 + .../linux-with-die/cpu/cpu1/topology/die_cpus | 1 + .../cpu/cpu1/topology/die_cpus_list | 1 + .../linux-with-die/cpu/cpu1/topology/die_id | 1 + .../cpu/cpu1/topology/package_cpus| 1 + .../cpu/cpu1/topology/package_cpus_list | 1 + .../cpu/cpu1/topology/physical_package_id | 1 + .../cpu/cpu1/topology/thread_siblings | 1 + .../cpu/cpu1/topology/thread_siblings_list| 1 + .../linux-with-die/cpu/cpu10/online | 1 + .../cpu/cpu10/topology/core_cpus | 1 + .../cpu/cpu10/topology/core_cpus_list | 1 + .../linux-with-die/cpu/cpu10/topology/core_id | 1 + .../cpu/cpu10/topology/core_siblings | 1 + .../cpu/cpu10/topology/core_siblings_list | 1 + .../cpu/cpu10/topology/die_cpus | 1 + .../cpu/cpu10/topology/die_cpus_list | 1 + .../linux-with-die/cpu/cpu10/topology/die_id | 1 + .../cpu/cpu10/topology/package_cpus | 1 + .../cpu/cpu10/topology/package_cpus_list | 1 + .../cpu/cpu10/topology/physical_package_id| 1 + .../cpu/cpu10/topology/thread_siblings| 1 + .../cpu/cpu10/topology/thread_siblings_list | 1 + .../linux-with-die/cpu/cpu11/online | 1 + .../cpu/cpu11/topology/core_cpus | 1 + .../cpu/cpu11/topology/core_cpus_list | 1 + .../linux-with-die/cpu/cpu11/topology/core_id | 1 + .../cpu/cpu11/topology/core_siblings | 1 + .../cpu/cpu11/topology/core_siblings_list | 1 + .../cpu/cpu11/topology/die_cpus | 1 + .../cpu/cpu11/topology/die_cpus_list | 1 + .../linux-with-die/cpu/cpu11/topology/die_id | 1 + .../cpu/cpu11/topology/package_cpus | 1 + .../cpu/cpu11/topology/package_cpus_list | 1 + .../cpu/cpu11/topology/physical_package_id| 1 + .../cpu/cpu11/topology/thread_siblings| 1 + .../cpu/cpu11/topology/thread_siblings_list | 1 + .../linux-with-die/cpu/cpu2/online| 1 + .../cpu/cpu2/topology/core_cpus | 1 + .../cpu/cpu2/topology/core_cpus_list | 1 + .../linux-with-die/cpu/cpu2/topology/core_id | 1 + .../cpu/cpu2/topology/core_siblings | 1 + .../cpu/cpu2/topology/core_siblings_list | 1 + .../linux-with-die/cpu/cpu2/topology/die_cpus | 1 + .../cpu/cpu2/topology/die_cpus_list | 1 + .../linux-with-die/cpu/cpu2/topology/die_id | 1 + .../cpu/cpu2/topology/package_cpus| 1 + .../cpu/cpu2/topology/package_cpus_list | 1 + .../cpu/cpu2/topology/physical_package_id | 1 + .../cpu/cpu2/topology/thread_siblings | 1 + .../cpu/cpu2/topology/thread_siblings_list| 1 + .../linux-with-die/cpu/cpu3/online| 1 + .../cpu/cpu3/topology/core_cpus | 1 + .../cpu/cpu3/topology/core_cpus_list | 1 + .../linux-with-die/cpu/cpu3/topology/core_id | 1 + .../cpu/cpu3/topology/core_siblings | 1 + .../cpu/cpu3/topology/core_siblings_list | 1 + .../linux-with-die/cpu/cpu3/topology/die_cpus | 1 + .../cpu/cpu3/topology/die_cpus_list | 1 + .../linux-with-die/cpu/cpu3/topology/die_id | 1 +
[libvirt] [PATCH 0/5] introduce support for CPU dies in host/guest topology
Latest generation CPUs (CascadeLake-AP) support a new topology level known as a 'die', sitting between a socket and a core. QEMU supports this with -smp arg since 4.1.0 Linux can report this via /sys/devices/system/cpu/cpuNNN/topology via 'die_id' and 'die_cpus' and 'die_cpus_list' files since 5.2 This series adds support for in guest XML to have a new 'dies' parameter, passed to QEMU, which defaults to '1' if omitted. It extends host capabilities so that NUMA topology reports a new 'die_id' attribute We can't expand 'virNodeInfoPtr' struct to have a die field, so this will remain forever reporting 'cores' as being 'dies * cores'. The in host capabilities XML is an interesting question. If we are strict with our API semantics we would *not* add a 'dies' parameter with any value other than '1' to in the host capabilities. If we reported a value other than 1, then any existing apps which multiple sockets*cores*threads will get the wrong total CPU count. We already know is broken by design for asymetric hardware, so we could simply document that it will forever be broken wrt to CPU dies too. In this case we might be better to not even report 'dies=1', just leave out the attr entirely. Interestingly though, is already more broken than it should be. For a VM with -smp 12,sockets=2,dies=3,cores=2,threads=1 it is reporting . It should at least do . I suspect the presence of dies is confusing the really incredibly horrible logic in virHostCPUGetInfoLinux. This will also impact virNodeInfoPtr data. So even if we don't report dies, I think we still have a bug that needs fixing here for the coarse node topology. I'm also confused about what I see with EPYC. IIUC, it was supposed to use the 'dies' concept, but in machines I've tested, Linux never reports die count other than 1. Perhaps only certain EPYC CPU models or generations use 'dies', or perhaps Linux isn't reporting correctly for EPYC, or perhaps I'm mislead into believeing it uses dies. Anyway, the upshot is I've not found any real hardware to test this series on. I've tested it only inside a QEMU guest with the suitable -smp arg to fake dies. Daniel P. Berrangé (5): conf: add support for specifying CPU "dies" parameter conf: remove unused virCapabilitiesSetHostCPU method qemu: add support for specifying CPU "dies" topology parameter hostcpu: add support for reporting die_id in NUMA topology tests: add host CPU data files for validating die_id docs/formatcaps.html.in | 2 +- docs/formatdomain.html.in | 14 +- docs/schemas/capability.rng | 3 + docs/schemas/cputypes.rng | 5 + src/bhyve/bhyve_command.c | 5 + src/conf/capabilities.c | 26 +- src/conf/capabilities.h | 7 +- src/conf/cpu_conf.c | 18 +- src/conf/cpu_conf.h | 1 + src/conf/domain_conf.c| 3 +- src/cpu/cpu.c | 1 + src/libvirt_linux.syms| 1 + src/libvirt_private.syms | 1 - src/libxl/libxl_capabilities.c| 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 12 +- src/util/virhostcpu.c | 16 + src/util/virhostcpu.h | 1 + src/vmx/vmx.c | 7 + .../x86_64-host+guest,model486-result.xml | 2 +- .../x86_64-host+guest,models-result.xml | 2 +- .../cputestdata/x86_64-host+guest-result.xml | 2 +- tests/cputestdata/x86_64-host+guest.xml | 2 +- .../x86_64-host+host-model-nofallback.xml | 2 +- ...t-Haswell-noTSX+Haswell,haswell-result.xml | 2 +- ...ell-noTSX+Haswell-noTSX,haswell-result.xml | 2 +- ...ost-Haswell-noTSX+Haswell-noTSX-result.xml | 2 +- .../x86_64-host-worse+guest-result.xml| 2 +- .../caps_4.1.0.x86_64.xml | 1 + .../caps_4.2.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 + .../qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 + .../caps_4.2.0.x86_64.xml | 1 + .../ppc64-modern-bulk-result-conf.xml | 2 +- .../ppc64-modern-bulk-result-live.xml | 2 +- .../ppc64-modern-individual-result-conf.xml | 2 +- .../ppc64-modern-individual-result-live.xml | 2 +- .../x86-modern-bulk-result-conf.xml | 2 +- .../x86-modern-bulk-result-live.xml | 2 +- .../x86-modern-individual-add-result-conf.xml | 2 +- .../x86-modern-individual-add-result-live.xml | 2 +- .../x86-old-bulk-result-conf.xml | 2 +- .../x86-old-bulk-result-live.xml | 2 +- .../cpu-hotplug-granularity.xml | 2 +-
[libvirt] [PATCH 2/5] conf: remove unused virCapabilitiesSetHostCPU method
Signed-off-by: Daniel P. Berrangé
---
src/conf/capabilities.c | 21 -
src/conf/capabilities.h | 6 --
src/libvirt_private.syms | 1 -
3 files changed, 28 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index da54591c11..e54e46f54e 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -368,27 +368,6 @@ virCapabilitiesHostNUMAAddCell(virCapsHostNUMAPtr caps,
g_ptr_array_add(caps->cells, cell);
}
-
-/**
- * virCapabilitiesSetHostCPU:
- * @caps: capabilities to extend
- * @cpu: CPU definition
- *
- * Sets host CPU specification
- */
-int
-virCapabilitiesSetHostCPU(virCapsPtr caps,
- virCPUDefPtr cpu)
-{
-if (cpu == NULL)
-return -1;
-
-caps->host.cpu = cpu;
-
-return 0;
-}
-
-
/**
* virCapabilitiesAllocMachines:
* @machines: machine variants for emulator ('pc', or 'isapc', etc)
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index f604e7b95e..4a49e94aa5 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -258,12 +258,6 @@ virCapabilitiesHostNUMAAddCell(virCapsHostNUMAPtr caps,
int npageinfo,
virCapsHostNUMACellPageInfoPtr pageinfo);
-
-int
-virCapabilitiesSetHostCPU(virCapsPtr caps,
- virCPUDefPtr cpu);
-
-
virCapsGuestMachinePtr *
virCapabilitiesAllocMachines(const char *const *names,
int nnames);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a7b1ef23bc..5799ac8270 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -77,7 +77,6 @@ virCapabilitiesHostSecModelAddBaseLabel;
virCapabilitiesInitCaches;
virCapabilitiesInitPages;
virCapabilitiesNew;
-virCapabilitiesSetHostCPU;
virCapabilitiesSetNetPrefix;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] conf: add support for specifying CPU "dies" parameter
Recently CPU hardware vendors have started to support a new level of inside the CPU package topology known as a "die". Thus the hierarchy is now: sockets > dies > cores > threads This adds support for "dies" in the XML parser, with the value defaulting to 1 if not specified for backwards compatibility. For example a system with 64 logical CPUs might report Signed-off-by: Daniel P. Berrangé --- docs/formatcaps.html.in| 2 +- docs/formatdomain.html.in | 14 +++--- docs/schemas/cputypes.rng | 5 + src/bhyve/bhyve_command.c | 5 + src/conf/cpu_conf.c| 18 -- src/conf/cpu_conf.h| 1 + src/conf/domain_conf.c | 3 ++- src/cpu/cpu.c | 1 + src/libxl/libxl_capabilities.c | 1 + src/qemu/qemu_command.c| 5 + src/vmx/vmx.c | 7 +++ .../x86_64-host+guest,model486-result.xml | 2 +- .../x86_64-host+guest,models-result.xml| 2 +- tests/cputestdata/x86_64-host+guest-result.xml | 2 +- tests/cputestdata/x86_64-host+guest.xml| 2 +- .../x86_64-host+host-model-nofallback.xml | 2 +- ...st-Haswell-noTSX+Haswell,haswell-result.xml | 2 +- ...well-noTSX+Haswell-noTSX,haswell-result.xml | 2 +- ...host-Haswell-noTSX+Haswell-noTSX-result.xml | 2 +- .../x86_64-host-worse+guest-result.xml | 2 +- .../ppc64-modern-bulk-result-conf.xml | 2 +- .../ppc64-modern-bulk-result-live.xml | 2 +- .../ppc64-modern-individual-result-conf.xml| 2 +- .../ppc64-modern-individual-result-live.xml| 2 +- .../x86-modern-bulk-result-conf.xml| 2 +- .../x86-modern-bulk-result-live.xml| 2 +- .../x86-modern-individual-add-result-conf.xml | 2 +- .../x86-modern-individual-add-result-live.xml | 2 +- .../x86-old-bulk-result-conf.xml | 2 +- .../x86-old-bulk-result-live.xml | 2 +- .../cpu-hotplug-granularity.xml| 2 +- tests/qemuxml2argvdata/cpu-hotplug-startup.xml | 2 +- tests/qemuxml2argvdata/cpu-numa-disjoint.xml | 2 +- tests/qemuxml2argvdata/cpu-numa-disordered.xml | 2 +- tests/qemuxml2argvdata/cpu-numa-memshared.xml | 2 +- .../cpu-numa-no-memory-element.xml | 2 +- tests/qemuxml2argvdata/cpu-numa1.xml | 2 +- tests/qemuxml2argvdata/cpu-numa2.xml | 2 +- tests/qemuxml2argvdata/cpu-numa3.xml | 2 +- tests/qemuxml2argvdata/cpu-topology1.xml | 2 +- tests/qemuxml2argvdata/cpu-topology2.xml | 2 +- tests/qemuxml2argvdata/cpu-topology3.xml | 2 +- .../fd-memory-no-numa-topology.xml | 2 +- .../fd-memory-numa-topology.xml| 2 +- .../fd-memory-numa-topology2.xml | 2 +- .../fd-memory-numa-topology3.xml | 2 +- .../graphics-spice-timeout.xml | 2 +- tests/qemuxml2argvdata/hugepages-nvdimm.xml| 2 +- .../memfd-memory-default-hugepage.xml | 2 +- tests/qemuxml2argvdata/memfd-memory-numa.xml | 2 +- tests/qemuxml2argvdata/memory-align-fail.xml | 2 +- .../memory-hotplug-dimm-addr.xml | 2 +- tests/qemuxml2argvdata/memory-hotplug-dimm.xml | 2 +- .../memory-hotplug-nvdimm-access.xml | 2 +- .../memory-hotplug-nvdimm-align.xml| 2 +- .../memory-hotplug-nvdimm-label.xml| 2 +- .../memory-hotplug-nvdimm-pmem.xml | 2 +- .../memory-hotplug-nvdimm-readonly.xml | 2 +- .../qemuxml2argvdata/memory-hotplug-nvdimm.xml | 2 +- tests/qemuxml2argvdata/memory-hotplug.xml | 2 +- .../numad-auto-memory-vcpu-cpuset.xml | 2 +- ...uto-memory-vcpu-no-cpuset-and-placement.xml | 2 +- .../numad-auto-vcpu-no-numatune.xml| 2 +- ...ad-auto-vcpu-static-numatune-no-nodeset.xml | 2 +- .../numad-auto-vcpu-static-numatune.xml| 2 +- .../numad-static-memory-auto-vcpu.xml | 2 +- .../numad-static-vcpu-no-numatune.xml | 2 +- tests/qemuxml2argvdata/numad.xml | 2 +- .../numatune-auto-nodeset-invalid.xml | 2 +- .../numatune-memory-invalid-nodeset.xml| 2 +- tests/qemuxml2argvdata/numatune-memory.xml | 2 +- .../pci-expander-bus-bad-machine.xml | 2 +- tests/qemuxml2argvdata/pci-expander-bus.xml| 2 +- .../pcie-expander-bus-bad-bus.xml | 2 +- .../pcie-expander-bus-bad-machine.xml | 2 +- tests/qemuxml2argvdata/pcie-expander-bus.xml | 2 +- .../pseries-default-phb-numa-node.xml | 2 +- .../qemuxml2argvdata/pseries-phb-numa-node.xml | 2 +- tests/qemuxml2argvdata/smp.xml | 2 +- tests/qemuxml2xmloutdata/cpu-numa-disjoint.xml | 2 +- .../qemuxml2xmloutdata/cpu-numa-disordered.xml | 2 +-
Re: [libvirt] [PATCH v2 1/2] virhostuptime: Introduce virHostBootTimeInit()
On Thu, Dec 19, 2019 at 12:25:00PM +0100, Michal Privoznik wrote:
> The idea is to offer callers an init function that they can call
> independently to ensure that the global variables get
> initialized.
>
> Signed-off-by: Michal Privoznik
> ---
> src/libvirt_private.syms | 1 +
> src/util/virhostuptime.c | 13 -
> src/util/virhostuptime.h | 3 +++
> 3 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index a7b1ef23bc..257ce00615 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2194,6 +2194,7 @@ virHostMemSetParameters;
>
>
> # util/virhostuptime.h
> +virHostBootTimeInit;
> virHostGetBootTime;
>
>
> diff --git a/src/util/virhostuptime.c b/src/util/virhostuptime.c
> index 8c49c3d40e..6c251c0d2d 100644
> --- a/src/util/virhostuptime.c
> +++ b/src/util/virhostuptime.c
> @@ -126,7 +126,8 @@ virHostGetBootTimeOnceInit(void)
> int
> virHostGetBootTime(unsigned long long *when)
> {
> -if (virOnce(, virHostGetBootTimeOnceInit) < 0)
> +if (bootTime == 0 &&
> +virHostBootTimeInit() < 0)
The bootTime==0 check is not required & technical a data race.
virHostBootTimeInit should be a no-op if it has already been
invoked
> return -1;
>
> if (bootTimeErrno) {
> @@ -137,3 +138,13 @@ virHostGetBootTime(unsigned long long *when)
> *when = bootTime;
> return 0;
> }
> +
> +
> +int
> +virHostBootTimeInit(void)
> +{
> +if (virOnce(, virHostGetBootTimeOnceInit) < 0)
> +return -1;
> +
> +return 0;
> +}
> diff --git a/src/util/virhostuptime.h b/src/util/virhostuptime.h
> index 7e2c4c0c81..1ac638fd6e 100644
> --- a/src/util/virhostuptime.h
> +++ b/src/util/virhostuptime.h
> @@ -25,3 +25,6 @@
> int
> virHostGetBootTime(unsigned long long *when)
> G_GNUC_NO_INLINE;
> +
> +int
> +virHostBootTimeInit(void);
> --
> 2.24.1
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] remote_daemon: Log host boot time
On 12/18/19 11:07 PM, Cole Robinson wrote:
> On 12/17/19 8:25 AM, Michal Privoznik wrote:
>> This is not strictly needed, but it makes sure we initialize the
>> @bootTime global variable. Thing is, in order to validate XATTRs
>> and prune those set in some previous runs of the host, a
>> timestamp is recorded in XATTRs. The host boot time was unique
>> enough so it was chosen as the timestamp value. And to avoid
>> querying and parsing /proc/uptime every time, the query function
>> does that only once and stores the boot time in a global
>> variable. However, the only time the query function is called is
>> in a child process that does lock files and changes seclabels. So
>> effectively, we are doing exactly what we wanted to prevent from
>> happening.
>>
>> The fix is simple, call the query function whilst the daemon is
>> initializing.
>>
>> Signed-off-by: Michal Privoznik
>> ---
>>
>> Since this will be used by security driver only, I was tempted to put
>> this somewhere under src/security/, but especially because of split
>> daemon I didn't. Placing this into remote_daemon.c makes sure all
>> sub-daemons will have the variable initialized and won't suffer the
>> problem.
>>
>> src/remote/remote_daemon.c | 8
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
>> index b400b1dd10..5debb6ce97 100644
>> --- a/src/remote/remote_daemon.c
>> +++ b/src/remote/remote_daemon.c
>> @@ -57,6 +57,7 @@
>> #include "virgettext.h"
>> #include "util/virnetdevopenvswitch.h"
>> #include "virsystemd.h"
>> +#include "virhostuptime.h"
>>
>> #include "driver.h"
>>
>> @@ -1020,6 +1021,7 @@ int main(int argc, char **argv) {
>> bool implicit_conf = false;
>> char *run_dir = NULL;
>> mode_t old_umask;
>> +unsigned long long bootTime;
>>
>> struct option opts[] = {
>> { "verbose", no_argument, , 'v'},
>> @@ -1151,6 +1153,12 @@ int main(int argc, char **argv) {
>> exit(EXIT_FAILURE);
>> }
>>
>> +if (virHostGetBootTime() < 0) {
>> +VIR_DEBUG("Unable to get host boot time");
>> +} else {
>> +VIR_DEBUG("host boot time: %lld", bootTime);
>> +}
>> +
>
> Please add a comment that this is initializing some global state that we
> need elsewhere, because otherwise it won't be obvious why this is here.
> With that:
>
> Reviewed-by: Cole Robinson
>
> Better IMO would be have an explicit virHostBootTimeInit function, and
> any usage of GetBootTime would fail if the init hasn't been called yet.
> It would make this case more self descriptive, and make sure any new
> users aren't doing it wrong
Agreed, patches proposed here:
https://www.redhat.com/archives/libvir-list/2019-December/msg01244.html
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/2] remote_daemon: Initialize host boot time global variable
This is not strictly needed, but it makes sure we initialize the
@bootTime global variable. Thing is, in order to validate XATTRs
and prune those set in some previous runs of the host, a
timestamp is recorded in XATTRs. The host boot time was unique
enough so it was chosen as the timestamp value. And to avoid
querying and parsing /proc/uptime every time, the query function
does that only once and stores the boot time in a global
variable. However, the only time the query function is called is
in a child process that does lock files and changes seclabels. So
effectively, we are doing exactly what we wanted to prevent from
happening.
The fix is simple, call the virHostBootTimeInit() function which
sets the global variable.
Signed-off-by: Michal Privoznik
---
src/remote/remote_daemon.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index b400b1dd10..27b538b292 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -57,6 +57,7 @@
#include "virgettext.h"
#include "util/virnetdevopenvswitch.h"
#include "virsystemd.h"
+#include "virhostuptime.h"
#include "driver.h"
@@ -1151,6 +1152,14 @@ int main(int argc, char **argv) {
exit(EXIT_FAILURE);
}
+/* Let's try to initialize global variable that holds the host's boot
time. */
+if (virHostBootTimeInit() < 0) {
+/* This is acceptable failure. Maybe we won't need the boot time
+ * anyway, and if we do, then virHostGetBootTime() returns an
+ * appropriate error. */
+VIR_DEBUG("Ignoring failed boot time init");
+}
+
daemonSetupNetDevOpenvswitch(config);
if (daemonSetupAccessManager(config) < 0) {
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/2] virhostuptime: Introduce virHostBootTimeInit()
The idea is to offer callers an init function that they can call
independently to ensure that the global variables get
initialized.
Signed-off-by: Michal Privoznik
---
src/libvirt_private.syms | 1 +
src/util/virhostuptime.c | 13 -
src/util/virhostuptime.h | 3 +++
3 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a7b1ef23bc..257ce00615 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2194,6 +2194,7 @@ virHostMemSetParameters;
# util/virhostuptime.h
+virHostBootTimeInit;
virHostGetBootTime;
diff --git a/src/util/virhostuptime.c b/src/util/virhostuptime.c
index 8c49c3d40e..6c251c0d2d 100644
--- a/src/util/virhostuptime.c
+++ b/src/util/virhostuptime.c
@@ -126,7 +126,8 @@ virHostGetBootTimeOnceInit(void)
int
virHostGetBootTime(unsigned long long *when)
{
-if (virOnce(, virHostGetBootTimeOnceInit) < 0)
+if (bootTime == 0 &&
+virHostBootTimeInit() < 0)
return -1;
if (bootTimeErrno) {
@@ -137,3 +138,13 @@ virHostGetBootTime(unsigned long long *when)
*when = bootTime;
return 0;
}
+
+
+int
+virHostBootTimeInit(void)
+{
+if (virOnce(, virHostGetBootTimeOnceInit) < 0)
+return -1;
+
+return 0;
+}
diff --git a/src/util/virhostuptime.h b/src/util/virhostuptime.h
index 7e2c4c0c81..1ac638fd6e 100644
--- a/src/util/virhostuptime.h
+++ b/src/util/virhostuptime.h
@@ -25,3 +25,6 @@
int
virHostGetBootTime(unsigned long long *when)
G_GNUC_NO_INLINE;
+
+int
+virHostBootTimeInit(void);
--
2.24.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/2] Initialize host boot time global var upfront
This is a v2 for: https://www.redhat.com/archives/libvir-list/2019-December/msg01045.html While technically v1 was fixed I agree with Cole's suggestion, so I'm discarding v1 and sending another version which implements his suggestion. Michal Prívozník (2): virhostuptime: Introduce virHostBootTimeInit() remote_daemon: Initialize host boot time global variable src/libvirt_private.syms | 1 + src/remote/remote_daemon.c | 9 + src/util/virhostuptime.c | 13 - src/util/virhostuptime.h | 3 +++ 4 files changed, 25 insertions(+), 1 deletion(-) -- 2.24.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/7] libvirt: support an "embed" URI path selector for opening drivers
On Tue, Dec 17, 2019 at 12:41:27PM -0500, Cole Robinson wrote:
> On 12/2/19 10:03 AM, Daniel P. Berrangé wrote:
> > The driver URI scheme:
> >
> > "$drivername:///embed?root=/some/path"
> >
> > enables a new way to use the drivers by embedding them directly in the
> > calling process. To use this the process must have a thread running the
> > libvirt event loop. This URI will then cause libvirt to dynamically load
> > the driver module and call its global initialization function. This
> > syntax is applicable to any driver, but only those will have been
> > modified to support a custom root directory and embed URI path will
> > successfully open.
> >
> > The application can now make normal libvirt API calls which are all
> > serviced in-process with no RPC layer involved.
> >
> > It is required to specify an explicit root directory, and locks will be
> > acquired on this directory to avoid conflicting with another app that
> > might accidentally pick the same directory.
> >
> > Use of '/' is not explicitly forbidden, but note that the file layout
> > used underneath the embedded driver root does not match the file
> > layout used by system/session mode drivers. So this cannot be used as
> > a backdoor to interact with, or fake, the system/session mode drivers.
> >
> > Libvirt will create arbitrary files underneath this root directory. The
> > root directory can be kept untouched across connection open attempts if
> > the application needs persistence. The application is responsible for
> > purging everything underneath this root directory when finally no longer
> > required.
> >
> > Even when a virt driver is used in embedded mode, it is still possible
> > for it to in turn use functionality that calls out to other secondary
> > drivers in libvirtd. For example an embedded instance of QEMU can open
> > the network, secret or storage drivers in the system libvirtd.
> >
> > That said, the application would typically want to at least open an
> > embedded secret driver ("secret:///embed?root=/some/path"). Note that
> > multiple different embedded drivers can use the same root prefix and
> > co-operate just as they would inside a normal libvirtd daemon.
> >
> > A key thing to note is that for this to work, the application that links
> > to libvirt *MUST* be built with -Wl,--export-dynamic to ensure that
> > symbols from libvirt.so are exported & thus available to the dynamically
> > loaded driver module. If libvirt.so itself was dynamically loaded then
> > RTLD_GLOBAL must be passed to dlopen().
> >
> > Signed-off-by: Daniel P. Berrangé
> > ---
> > src/driver-state.h | 1 +
> > src/driver.h | 2 ++
> > src/libvirt.c | 72 --
> > 3 files changed, 73 insertions(+), 2 deletions(-)
> It would be nice if this logic was moved to a separate function
>
> I've hit a couple issues in testing, not sure if/where the fixes will
> live, so I'll just mention them here. Also the reviewed patches are
> pushable IMO
>
> I tried this code:
>
> from gi.repository import LibvirtGLib
>
> import libvirt
>
>
>
> LibvirtGLib.init(None)
>
> LibvirtGLib.event_register()
>
>
>
> conn1 = libvirt.open("qemu:///embed?root=/tmp/foo")
>
> conn2 = libvirt.open("qemu:///embed?root=/tmp/bar")
>
> print(conn1.listAllDomains())
>
> print(conn2.listAllDomains())
>
>
> With /tmp/foo populated with a VM: both connections see the same values.
> So this should be rejected. Even trying to close conn1 fully and open a
> new embed root will only see the old root. So maybe this needs knowledge
> in the driver lookup.
Right, so because of our design with the single global driver struct, we
can only have 1 instance of the driver active per process. So yeah, we
need to enforce this in some place or other.
It should be valid to open multiple connections to the same embedded
driver instance directory though.
> The second issue: testing with virt-manager, everything locks up with
> OpenGraphicsFD:
>
> #0 0x77a7f07a in pthread_cond_timedwait@@GLIBC_2.3.2 () at
> /lib64/libpthread.so.0
> #1 0x7fffe94be113 in virCondWaitUntil (c=c@entry=0x7fffc8071f98,
> m=m@entry=0x7fffc8071ed0, whenms=whenms@entry=1576602286073) at
> /home/crobinso/src/libvirt/src/util/virthread.c:159
> #2 0x7fffe44fc549 in qemuDomainObjBeginJobInternal
> (driver=driver@entry=0x7fffc8004f30, obj=0x7fffc8071ec0,
> job=job@entry=QEMU_JOB_MODIFY,
> agentJob=agentJob@entry=QEMU_AGENT_JOB_NONE,
> asyncJob=asyncJob@entry=QEMU_ASYNC_JOB_NONE, nowait=nowait@entry=false)
> at /home/crobinso/src/libvirt/src/qemu/qemu_domain.c:9357
> #3 0x7fffe4500aa1 in qemuDomainObjBeginJob
> (driver=driver@entry=0x7fffc8004f30, obj=,
> job=job@entry=QEMU_JOB_MODIFY) at
> /home/crobinso/src/libvirt/src/qemu/qemu_domain.c:9521
> #4 0x7fffe4582572 in qemuDomainOpenGraphicsFD (dom=,
> idx=, flags=0) at
> /home/crobinso/src/libvirt/src/qemu/qemu_driver.c:18990
> #5 0x7fffe968699c in
[libvirt] [PATCH v2 1/1] qemu: hide details of fake reboot
If we use fake reboot then domain goes thru running->shutdown->running
state changes with shutdown state only for short period of time. At
least this is implementation details leaking into API. And also there is
one real case when this is not convinient. I'm doing a backup with the
help of temporary block snapshot (with the help of qemu's API which is
used in the newly created libvirt's backup API). If guest is shutdowned
I want to continue to backup so I don't kill the process and domain is
in shutdown state. Later when backup is finished I want to destroy qemu
process. So I check if it is in shutdowned state and destroy it if it
is. Now if instead of shutdown domain got fake reboot then I can destroy
process in the middle of fake reboot process.
After shutdown event we also get stop event and now as domain state is
running it will be transitioned to paused state and back to running
later. Though this is not critical for the described case I guess it is
better not to leak these details to user too. So let's leave domain in
running state on stop event if fake reboot is in process.
Reconnection code handles this patch without modification. It detects
that qemu is not running due to shutdown and then calls
qemuProcessShutdownOrReboot
which reboots as fake reboot flag is set.
Signed-off-by: Nikolay Shirokovskiy
---
Changes from v1[1]:
- rebase on current master
- add comments
- use special flag to check if we should go paused or not*
- add notes about reconnection to commit message
* Using just fake reboot flag is not reliable. What if ACPI shutdown is
ignored by guest? Reboot flag will remain set and now domain state
will remain running on plain pause.
[1] https://www.redhat.com/archives/libvir-list/2019-October/msg01827.html
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_process.c | 61 -
2 files changed, 37 insertions(+), 25 deletions(-)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index a32852047c..a39b9546ae 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -319,6 +319,7 @@ struct _qemuDomainObjPrivate {
char *lockState;
bool fakeReboot;
+bool pausedShutdown;
virTristateBool allowReboot;
int jobs_queued;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7e1db50e8f..3e5fe3b6de 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -505,6 +505,7 @@ qemuProcessFakeReboot(void *opaque)
qemuDomainObjEndJob(driver, vm);
cleanup:
+priv->pausedShutdown = false;
if (ret == -1)
ignore_value(qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_FORCE));
virDomainObjEndAPI();
@@ -528,6 +529,7 @@ qemuProcessShutdownOrReboot(virQEMUDriverPtr driver,
vm) < 0) {
VIR_ERROR(_("Failed to create reboot thread, killing domain"));
ignore_value(qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_NOWAIT));
+priv->pausedShutdown = false;
virObjectUnref(vm);
}
} else {
@@ -589,35 +591,41 @@ qemuProcessHandleShutdown(qemuMonitorPtr mon
G_GNUC_UNUSED,
goto unlock;
}
-VIR_DEBUG("Transitioned guest %s to shutdown state",
- vm->def->name);
-virDomainObjSetState(vm,
- VIR_DOMAIN_SHUTDOWN,
- VIR_DOMAIN_SHUTDOWN_UNKNOWN);
+/* In case of fake reboot qemu shutdown state is transient so don't
+ * change domain state nor send events. */
+if (!priv->fakeReboot) {
+VIR_DEBUG("Transitioned guest %s to shutdown state",
+ vm->def->name);
+virDomainObjSetState(vm,
+ VIR_DOMAIN_SHUTDOWN,
+ VIR_DOMAIN_SHUTDOWN_UNKNOWN);
-switch (guest_initiated) {
-case VIR_TRISTATE_BOOL_YES:
-detail = VIR_DOMAIN_EVENT_SHUTDOWN_GUEST;
-break;
+switch (guest_initiated) {
+case VIR_TRISTATE_BOOL_YES:
+detail = VIR_DOMAIN_EVENT_SHUTDOWN_GUEST;
+break;
-case VIR_TRISTATE_BOOL_NO:
-detail = VIR_DOMAIN_EVENT_SHUTDOWN_HOST;
-break;
+case VIR_TRISTATE_BOOL_NO:
+detail = VIR_DOMAIN_EVENT_SHUTDOWN_HOST;
+break;
-case VIR_TRISTATE_BOOL_ABSENT:
-case VIR_TRISTATE_BOOL_LAST:
-default:
-detail = VIR_DOMAIN_EVENT_SHUTDOWN_FINISHED;
-break;
-}
+case VIR_TRISTATE_BOOL_ABSENT:
+case VIR_TRISTATE_BOOL_LAST:
+default:
+detail = VIR_DOMAIN_EVENT_SHUTDOWN_FINISHED;
+break;
+}
-event = virDomainEventLifecycleNewFromObj(vm,
- VIR_DOMAIN_EVENT_SHUTDOWN,
- detail);
+event = virDomainEventLifecycleNewFromObj(vm,
+ VIR_DOMAIN_EVENT_SHUTDOWN,
+ detail);
-
Re: [libvirt] [PATCH] cpu: add CLZERO CPUID support for AMD platforms (libvirt 4.5)
Thanks. Going forward I will only send patches against the current master.
Ani
On 12/16/19, 5:37 PM, "Jiri Denemark" wrote:
On Tue, Dec 03, 2019 at 03:19:39 -0800, Ani Sinha wrote:
> Qemu commit e900135dcfb67 ("i386: Add CPUID bit for CLZERO and
XSAVEERPTR")
> adds support for CLZERO CPUID bit. This commit extends support for this
> bit in libvirt.
>
> This patch is based off of libvirt-4.5 tree.
Thanks, but we are really only interested in patches against current
master. Good thing is you sent it too so I reviewed that version.
Jirka
--
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
