4:10 a.m.
The comment for that option states that the default value is 'none' but
it was not set by the code.
Fixes: 700450449377be4bf923e91d00f8fe8cf0975f66
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_conf.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 77fd7f6df7..68b086be54 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -295,6 +295,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged,
&cfg->nfirmwares) < 0)
return NULL;
+ cfg->deprecationBehavior = g_strdup("none");
+
return g_steal_pointer(&cfg);
}
--
2.30.2
5:07 a.m.
On 4/13/21 11:10 AM, Pavel Hrdina wrote:
The comment for that option states that the default value is
'none' but
it was not set by the code.
Fixes: 700450449377be4bf923e91d00f8fe8cf0975f66
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_conf.c | 2 ++
1 file changed, 2 insertions(+)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
5:53 a.m.
On Tue, Apr 13, 2021 at 11:10:18 +0200, Pavel Hrdina wrote:
The comment for that option states that the default value is
'none' but
it was not set by the code.
Fixes: 700450449377be4bf923e91d00f8fe8cf0975f66
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
Could you please elaborate what the problem of not setting it is?
'none' the same as NULL should result in doing nothing.
6:03 a.m.
On 4/13/21 12:53 PM, Peter Krempa wrote:
On Tue, Apr 13, 2021 at 11:10:18 +0200, Pavel Hrdina wrote:
> The comment for that option states that the default value is 'none' but
> it was not set by the code.
>
> Fixes: 700450449377be4bf923e91d00f8fe8cf0975f66
> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> ---
Could you please elaborate what the problem of not setting it is?
'none' the same as NULL should result in doing nothing.
Is it? In qemuBuildCompatDeprecatedCommandLine() a string is initialized
to cfg->deprecationBehavior and then passed to TypeFromString() which
fails and thus VIR_WARN() is printed. I believe that's where crash can
occur.
Michal
6:08 a.m.
On Tue, Apr 13, 2021 at 13:03:44 +0200, Michal Privoznik wrote:
On 4/13/21 12:53 PM, Peter Krempa wrote:
> On Tue, Apr 13, 2021 at 11:10:18 +0200, Pavel Hrdina wrote:
> > The comment for that option states that the default value is 'none'
but
> > it was not set by the code.
> >
> > Fixes: 700450449377be4bf923e91d00f8fe8cf0975f66
> > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> > ---
>
> Could you please elaborate what the problem of not setting it is?
>
> 'none' the same as NULL should result in doing nothing.
>
Is it? In qemuBuildCompatDeprecatedCommandLine() a string is initialized to
cfg->deprecationBehavior and then passed to TypeFromString() which fails and
thus VIR_WARN() is printed. I believe that's where crash can occur.
Ah, right. That's the stuff that should be in the commit message then.
6:09 a.m.
On Tue, Apr 13, 2021 at 12:53:23PM +0200, Peter Krempa wrote:
On Tue, Apr 13, 2021 at 11:10:18 +0200, Pavel Hrdina wrote:
> The comment for that option states that the default value is 'none' but
> it was not set by the code.
>
> Fixes: 700450449377be4bf923e91d00f8fe8cf0975f66
> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> ---
Could you please elaborate what the problem of not setting it is?
'none' the same as NULL should result in doing nothing.
When the config is not set in qemu.conf libvirt will print the following
warning if there is any deprecated command used:
warning : qemuBuildCompatDeprecatedCommandLine:10393 : Unsupported deprecation behavior
'(null)' for VM 'test'
So no NULL and 'none' are not the same.
We could fix qemuBuildCompatDeprecatedCommandLine() but I figured that
this is what we do with other options so that would be preferred
solution.
Pavel
6:13 a.m.
On Tue, Apr 13, 2021 at 13:09:15 +0200, Pavel Hrdina wrote:
On Tue, Apr 13, 2021 at 12:53:23PM +0200, Peter Krempa wrote:
> On Tue, Apr 13, 2021 at 11:10:18 +0200, Pavel Hrdina wrote:
> > The comment for that option states that the default value is 'none'
but
> > it was not set by the code.
> >
> > Fixes: 700450449377be4bf923e91d00f8fe8cf0975f66
> > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> > ---
>
> Could you please elaborate what the problem of not setting it is?
>
> 'none' the same as NULL should result in doing nothing.
When the config is not set in qemu.conf libvirt will print the following
warning if there is any deprecated command used:
warning : qemuBuildCompatDeprecatedCommandLine:10393 : Unsupported deprecation behavior
'(null)' for VM 'test'
So no NULL and 'none' are not the same.
We could fix qemuBuildCompatDeprecatedCommandLine() but I figured that
this is what we do with other options so that would be preferred
solution.
Sure. I agree, but as I've noted in reply to Michal, such explanation
should be part of the commit message.
6:16 a.m.
On Tue, Apr 13, 2021 at 01:13:41PM +0200, Peter Krempa wrote:
On Tue, Apr 13, 2021 at 13:09:15 +0200, Pavel Hrdina wrote:
> On Tue, Apr 13, 2021 at 12:53:23PM +0200, Peter Krempa wrote:
> > On Tue, Apr 13, 2021 at 11:10:18 +0200, Pavel Hrdina wrote:
> > > The comment for that option states that the default value is
'none' but
> > > it was not set by the code.
> > >
> > > Fixes: 700450449377be4bf923e91d00f8fe8cf0975f66
> > > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> > > ---
> >
> > Could you please elaborate what the problem of not setting it is?
> >
> > 'none' the same as NULL should result in doing nothing.
>
> When the config is not set in qemu.conf libvirt will print the following
> warning if there is any deprecated command used:
>
> warning : qemuBuildCompatDeprecatedCommandLine:10393 : Unsupported deprecation
behavior '(null)' for VM 'test'
>
> So no NULL and 'none' are not the same.
>
> We could fix qemuBuildCompatDeprecatedCommandLine() but I figured that
> this is what we do with other options so that would be preferred
> solution.
Sure. I agree, but as I've noted in reply to Michal, such explanation
should be part of the commit message.
I figured that what I wrote into commit message was good enough, I'll
add the warning message in the commit message before pushing.
Pavel
1319
days inactive
1319
days old
7 comments
3 participants
participants (3)
-
Michal Privoznik -
Pavel Hrdina -
Peter Krempa