[PATCH] libxl: Add lock process indicator to saved VM state

Jim Fehlig posted 1 patch 2 years, 2 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20220125021657.21768-1-jfehlig@suse.com
src/libxl/libxl_domain.c | 4 ++++
1 file changed, 4 insertions(+)
[PATCH] libxl: Add lock process indicator to saved VM state
Posted by Jim Fehlig 2 years, 2 months ago
Commit fa58f571ee added a lock processes indicator to the
libxlDomainObjPrivate struct to note that a lock process was
successfully started for the VM. However, the commit neglected to
add the indicator to the VM's saved state file. As a result, the
indicator is lost on libvirtd restart, along with the knowledge of
whether a lock process was started for the VM.

This change adds support for the indicator in the domainObjPrivate
data parse and format callbacks, ensuring its value survives libvirtd
restarts.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
 src/libxl/libxl_domain.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index feca60f7d2..8962adc60f 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -226,6 +226,7 @@ libxlDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
     libxlDomainObjPrivate *priv = vm->privateData;
 
     priv->lockState = virXPathString("string(./lockstate)", ctxt);
+    priv->lockProcessRunning = virXPathBoolean("count(./lockProcessRunning) > 0", ctxt);
 
     return 0;
 }
@@ -239,6 +240,9 @@ libxlDomainObjPrivateXMLFormat(virBuffer *buf,
     if (priv->lockState)
         virBufferAsprintf(buf, "<lockstate>%s</lockstate>\n", priv->lockState);
 
+    if (priv->lockProcessRunning)
+        virBufferAddLit(buf, "<lockProcessRunning/>\n");
+
     return 0;
 }
 
-- 
2.34.1


Re: [PATCH] libxl: Add lock process indicator to saved VM state
Posted by Michal Prívozník 2 years, 2 months ago
On 1/25/22 03:16, Jim Fehlig wrote:
> Commit fa58f571ee added a lock processes indicator to the
> libxlDomainObjPrivate struct to note that a lock process was
> successfully started for the VM. However, the commit neglected to
> add the indicator to the VM's saved state file. As a result, the
> indicator is lost on libvirtd restart, along with the knowledge of
> whether a lock process was started for the VM.
> 
> This change adds support for the indicator in the domainObjPrivate
> data parse and format callbacks, ensuring its value survives libvirtd
> restarts.
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
>  src/libxl/libxl_domain.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index feca60f7d2..8962adc60f 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -226,6 +226,7 @@ libxlDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
>      libxlDomainObjPrivate *priv = vm->privateData;
>  
>      priv->lockState = virXPathString("string(./lockstate)", ctxt);
> +    priv->lockProcessRunning = virXPathBoolean("count(./lockProcessRunning) > 0", ctxt);

Alternatively, virXPathBoolean("boolean(./lockProcessRunning)", ctxt);
should do the same. It may be less expensive to evaluate than count().

>  
>      return 0;
>  }
> @@ -239,6 +240,9 @@ libxlDomainObjPrivateXMLFormat(virBuffer *buf,
>      if (priv->lockState)
>          virBufferAsprintf(buf, "<lockstate>%s</lockstate>\n", priv->lockState);
>  
> +    if (priv->lockProcessRunning)
> +        virBufferAddLit(buf, "<lockProcessRunning/>\n");
> +
>      return 0;
>  }
>  

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

Michal

Re: [PATCH] libxl: Add lock process indicator to saved VM state
Posted by Jim Fehlig 2 years, 2 months ago
On 1/25/22 05:34, Michal Prívozník wrote:
> On 1/25/22 03:16, Jim Fehlig wrote:
>> Commit fa58f571ee added a lock processes indicator to the
>> libxlDomainObjPrivate struct to note that a lock process was
>> successfully started for the VM. However, the commit neglected to
>> add the indicator to the VM's saved state file. As a result, the
>> indicator is lost on libvirtd restart, along with the knowledge of
>> whether a lock process was started for the VM.
>>
>> This change adds support for the indicator in the domainObjPrivate
>> data parse and format callbacks, ensuring its value survives libvirtd
>> restarts.
>>
>> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
>> ---
>>   src/libxl/libxl_domain.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
>> index feca60f7d2..8962adc60f 100644
>> --- a/src/libxl/libxl_domain.c
>> +++ b/src/libxl/libxl_domain.c
>> @@ -226,6 +226,7 @@ libxlDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
>>       libxlDomainObjPrivate *priv = vm->privateData;
>>   
>>       priv->lockState = virXPathString("string(./lockstate)", ctxt);
>> +    priv->lockProcessRunning = virXPathBoolean("count(./lockProcessRunning) > 0", ctxt);
> 
> Alternatively, virXPathBoolean("boolean(./lockProcessRunning)", ctxt);
> should do the same. It may be less expensive to evaluate than count().

It looks better too. I made the change before pushing. Thanks for the suggestion.

Jim