qemu_process: Produce better debug message wrt domain namespaces

[PATCH] qemu_process: Produce better debug message wrt domain namespaces

Posted by Michal Privoznik 1 year, 2 months ago

When going through debug log of a domain startup process, one can
meet the following line:

  debug : qemuProcessLaunch:7668 : Building mount namespace

But this is in fact wrong. Firstly, domain namespaces are just
enabled in domain's privateData. Secondly, the debug message says
nothing about actual state of namespace - whether it was enabled
or not.

Therefore, move the debug printing into
qemuProcessEnableDomainNamespaces() and tweak it so that the
actual value is reflected.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_process.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e96b7689a3..68ced01ab2 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7378,11 +7378,17 @@ qemuProcessEnableDomainNamespaces(virQEMUDriver *driver,
                                   virDomainObj *vm)
 {
     g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+    const char *state = "disabled";
 
     if (virBitmapIsBitSet(cfg->namespaces, QEMU_DOMAIN_NS_MOUNT) &&
         qemuDomainEnableNamespace(vm, QEMU_DOMAIN_NS_MOUNT) < 0)
         return -1;
 
+    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
+        state = "enabled";
+
+    VIR_DEBUG("Mount namespace for domain name=%s are %s",
+              vm->def->name, state);
     return 0;
 }
 
@@ -7706,8 +7712,6 @@ qemuProcessLaunch(virConnectPtr conn,
 
     qemuDomainLogContextMarkPosition(logCtxt);
 
-    VIR_DEBUG("Building mount namespace");
-
     if (qemuProcessEnableDomainNamespaces(driver, vm) < 0)
         goto cleanup;
 
-- 
2.39.1

Re: [PATCH] qemu_process: Produce better debug message wrt domain namespaces

Posted by Jim Fehlig 1 year, 2 months ago

On 2/7/23 04:17, Michal Privoznik wrote:
> When going through debug log of a domain startup process, one can
> meet the following line:
> 
>    debug : qemuProcessLaunch:7668 : Building mount namespace
> 
> But this is in fact wrong. Firstly, domain namespaces are just
> enabled in domain's privateData. Secondly, the debug message says
> nothing about actual state of namespace - whether it was enabled
> or not.
> 
> Therefore, move the debug printing into
> qemuProcessEnableDomainNamespaces() and tweak it so that the
> actual value is reflected.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>   src/qemu/qemu_process.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index e96b7689a3..68ced01ab2 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -7378,11 +7378,17 @@ qemuProcessEnableDomainNamespaces(virQEMUDriver *driver,
>                                     virDomainObj *vm)
>   {
>       g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
> +    const char *state = "disabled";
>   
>       if (virBitmapIsBitSet(cfg->namespaces, QEMU_DOMAIN_NS_MOUNT) &&
>           qemuDomainEnableNamespace(vm, QEMU_DOMAIN_NS_MOUNT) < 0)
>           return -1;
>   
> +    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
> +        state = "enabled";
> +
> +    VIR_DEBUG("Mount namespace for domain name=%s are %s",
> +              vm->def->name, state);

Nit: s/are/is/ since 'namespace' is singular.

>       return 0;
>   }
>   
> @@ -7706,8 +7712,6 @@ qemuProcessLaunch(virConnectPtr conn,
>   
>       qemuDomainLogContextMarkPosition(logCtxt);
>   
> -    VIR_DEBUG("Building mount namespace");
> -
>       if (qemuProcessEnableDomainNamespaces(driver, vm) < 0)
>           goto cleanup;

Reviewed-by: Jim Fehlig <jfehlig@suse.com>

Regards,
Jim

Re: [PATCH] qemu_process: Produce better debug message wrt domain namespaces

Posted by Michal Prívozník 1 year, 2 months ago

On 2/7/23 19:30, Jim Fehlig wrote:
> On 2/7/23 04:17, Michal Privoznik wrote:
>> When going through debug log of a domain startup process, one can
>> meet the following line:
>>
>>    debug : qemuProcessLaunch:7668 : Building mount namespace
>>
>> But this is in fact wrong. Firstly, domain namespaces are just
>> enabled in domain's privateData. Secondly, the debug message says
>> nothing about actual state of namespace - whether it was enabled
>> or not.
>>
>> Therefore, move the debug printing into
>> qemuProcessEnableDomainNamespaces() and tweak it so that the
>> actual value is reflected.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>   src/qemu/qemu_process.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
>> index e96b7689a3..68ced01ab2 100644
>> --- a/src/qemu/qemu_process.c
>> +++ b/src/qemu/qemu_process.c
>> @@ -7378,11 +7378,17 @@
>> qemuProcessEnableDomainNamespaces(virQEMUDriver *driver,
>>                                     virDomainObj *vm)
>>   {
>>       g_autoptr(virQEMUDriverConfig) cfg =
>> virQEMUDriverGetConfig(driver);
>> +    const char *state = "disabled";
>>         if (virBitmapIsBitSet(cfg->namespaces, QEMU_DOMAIN_NS_MOUNT) &&
>>           qemuDomainEnableNamespace(vm, QEMU_DOMAIN_NS_MOUNT) < 0)
>>           return -1;
>>   +    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
>> +        state = "enabled";
>> +
>> +    VIR_DEBUG("Mount namespace for domain name=%s are %s",
>> +              vm->def->name, state);
> 
> Nit: s/are/is/ since 'namespace' is singular.

Yeah. I went back and forth on "namespaces are enabled/disabled" and
just as I decided on singular I forgot to read it again, whether the
sentence makes sense.

> 
>>       return 0;
>>   }
>>   @@ -7706,8 +7712,6 @@ qemuProcessLaunch(virConnectPtr conn,
>>         qemuDomainLogContextMarkPosition(logCtxt);
>>   -    VIR_DEBUG("Building mount namespace");
>> -
>>       if (qemuProcessEnableDomainNamespaces(driver, vm) < 0)
>>           goto cleanup;
> 
> Reviewed-by: Jim Fehlig <jfehlig@suse.com>

Thanks!

Michal