[PATCH] libxl: Fix driver reload

Jim Fehlig posted 1 patch 2 years, 7 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210913221051.1015-1-jfehlig@suse.com
src/libxl/libxl_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] libxl: Fix driver reload
Posted by Jim Fehlig 2 years, 7 months ago
On reload, the libxl driver calls virDomainObjListLoadAllConfigs to load
all configs from /etc/libvirt/libxl/ but incorrectly passes 'true' for
the liveStatus parameter, resulting in error messages such as

libvirtd[21053]: XML error: unexpected root element <domain>, expecting <domstatus>
libvirtd[21053]: Failed to load config for domain 'sles15sp3'

Fix by not requesting live status when re-reading the persistent VM config
files.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/libxl/libxl_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 6a3938ead4..c5dbcaafa5 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -821,7 +821,7 @@ libxlStateReload(void)
     virDomainObjListLoadAllConfigs(libxl_driver->domains,
                                    cfg->configDir,
                                    cfg->autostartDir,
-                                   true,
+                                   false,
                                    libxl_driver->xmlopt,
                                    NULL, libxl_driver);
 
-- 
2.33.0


Re: [PATCH] libxl: Fix driver reload
Posted by Michal Prívozník 2 years, 7 months ago
On 9/14/21 12:10 AM, Jim Fehlig wrote:
> On reload, the libxl driver calls virDomainObjListLoadAllConfigs to load
> all configs from /etc/libvirt/libxl/ but incorrectly passes 'true' for
> the liveStatus parameter, resulting in error messages such as
> 
> libvirtd[21053]: XML error: unexpected root element <domain>, expecting <domstatus>
> libvirtd[21053]: Failed to load config for domain 'sles15sp3'
> 
> Fix by not requesting live status when re-reading the persistent VM config
> files.
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
>  src/libxl/libxl_driver.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 6a3938ead4..c5dbcaafa5 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -821,7 +821,7 @@ libxlStateReload(void)
>      virDomainObjListLoadAllConfigs(libxl_driver->domains,
>                                     cfg->configDir,
>                                     cfg->autostartDir,
> -                                   true,
> +                                   false,
>                                     libxl_driver->xmlopt,
>                                     NULL, libxl_driver);
>  
> 

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

But I've noticed that on reload, below this hunk is a line that
autostarts domains. This is a deviation from other drivers - should we
autostart domains on daemon reload?

Michal

Re: [PATCH] libxl: Fix driver reload
Posted by Jim Fehlig 2 years, 7 months ago
On 9/14/21 04:23, Michal Prívozník wrote:
> On 9/14/21 12:10 AM, Jim Fehlig wrote:
>> On reload, the libxl driver calls virDomainObjListLoadAllConfigs to load
>> all configs from /etc/libvirt/libxl/ but incorrectly passes 'true' for
>> the liveStatus parameter, resulting in error messages such as
>>
>> libvirtd[21053]: XML error: unexpected root element <domain>, expecting <domstatus>
>> libvirtd[21053]: Failed to load config for domain 'sles15sp3'
>>
>> Fix by not requesting live status when re-reading the persistent VM config
>> files.
>>
>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>> ---
>>   src/libxl/libxl_driver.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index 6a3938ead4..c5dbcaafa5 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -821,7 +821,7 @@ libxlStateReload(void)
>>       virDomainObjListLoadAllConfigs(libxl_driver->domains,
>>                                      cfg->configDir,
>>                                      cfg->autostartDir,
>> -                                   true,
>> +                                   false,
>>                                      libxl_driver->xmlopt,
>>                                      NULL, libxl_driver);
>>   
>>
> 
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Thanks!

> But I've noticed that on reload, below this hunk is a line that
> autostarts domains. This is a deviation from other drivers - should we
> autostart domains on daemon reload?

No, we shouldn't. It appears to be mistakenly added when libxlAutostartDomain 
was introduced in commit fb92307f0d. I'll send a followup patch to remove it.

Regards,
Jim