[libvirt PATCH] qemu_conf: properly set 'deprecation_behavior' default value
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@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
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@redhat.com> --- src/qemu/qemu_conf.c | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
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@redhat.com> ---
Could you please elaborate what the problem of not setting it is? 'none' the same as NULL should result in doing nothing.
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@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
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@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.
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@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
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@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.
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@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
participants (3)
-
Michal Privoznik -
Pavel Hrdina -
Peter Krempa