[PATCH] support for hotplug/hotunplug in test hypervisor

2023-10-27 Thread Thanos Makatos 
Signed-off-by: Thanos Makatos 
---
 src/test/test_driver.c | 61 --
 1 file changed, 59 insertions(+), 2 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index e87d7cfd44..6eda0dcc01 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -26,8 +26,6 @@
 #include 
 #include 
 #include 
-
-
 #include "virerror.h"
 #include "datatypes.h"
 #include "test_driver.h"
@@ -10035,6 +10033,62 @@ testConnectGetDomainCapabilities(virConnectPtr conn 
G_GNUC_UNUSED,
 return virDomainCapsFormat(domCaps);
 }
 
+static int
+testVirDomainAttachDeviceFlags(virDomainPtr domain,
+   const char *xml,
+   unsigned int flags G_GNUC_UNUSED) {
+
+int ret = -1;
+virDomainObj *vm;
+testDriver *driver;
+virDomainDeviceDef *devConf;
+
+if (!(vm = testDomObjFromDomain(domain)))
+return -1;
+
+driver = domain->conn->privateData;
+
+if (!(devConf = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt,
+NULL, 0)))
+goto out;
+
+VIR_APPEND_ELEMENT(vm->def->hostdevs, vm->def->nhostdevs,
+devConf->data.hostdev);
+virDomainDeviceDefFree(devConf);
+ret = 0;
+out:
+virDomainObjEndAPI(&vm);
+return ret;
+}
+
+static int
+testVirDomainAttachDevice(virDomainPtr domain, const char *xml)
+{
+return testVirDomainAttachDeviceFlags(domain, xml, 0);
+}
+
+static int
+testVirDomainDetachDeviceAlias(virDomainPtr domain,
+   const char *alias,
+   unsigned int flags __attribute__((unused)))
+{
+virDomainObj *vm;
+int i;
+bool found = false;
+
+if (!(vm = testDomObjFromDomain(domain)))
+return -1;
+
+for (i = 0; i < vm->def->nhostdevs && !found; i++) {
+if (!strcmp(vm->def->hostdevs[i]->info->alias, alias)) {
+virDomainHostdevDefFree(vm->def->hostdevs[i]);
+VIR_DELETE_ELEMENT(vm->def->hostdevs, i, vm->def->nhostdevs);
+found = true;
+}
+}
+virDomainObjEndAPI(&vm);
+return found ? 0 : -1;
+}
 
 /*
  * Test driver
@@ -10058,6 +10112,9 @@ static virHypervisorDriver testHypervisorDriver = {
 .connectListDomains = testConnectListDomains, /* 0.1.1 */
 .connectNumOfDomains = testConnectNumOfDomains, /* 0.1.1 */
 .connectListAllDomains = testConnectListAllDomains, /* 0.9.13 */
+.domainAttachDevice = testVirDomainAttachDevice, /* 9.9.0 */
+.domainAttachDeviceFlags = testVirDomainAttachDeviceFlags, /* 9.9.0 */
+.domainDetachDeviceAlias = testVirDomainDetachDeviceAlias, /* 9.9.0 */
 .domainCreateXML = testDomainCreateXML, /* 0.1.4 */
 .domainCreateXMLWithFiles = testDomainCreateXMLWithFiles, /* 5.7.0 */
 .domainLookupByID = testDomainLookupByID, /* 0.1.1 */
-- 
2.27.0

RE: [PATCH] support for hotplug/hotunplug in test hypervisor

2023-10-30 Thread Thanos Makatos 
> -Original Message-
> From: Daniel P. Berrangé 
> Sent: Friday, October 27, 2023 6:13 PM
> To: Thanos Makatos 
> Cc: devel@lists.libvirt.org
> Subject: Re: [PATCH] support for hotplug/hotunplug in test hypervisor
> 
> On Fri, Oct 27, 2023 at 03:44:13PM +, Thanos Makatos wrote:
> > Signed-off-by: Thanos Makatos 
> > ---
> >  src/test/test_driver.c | 61
> --
> >  1 file changed, 59 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> > index e87d7cfd44..6eda0dcc01 100644
> > --- a/src/test/test_driver.c
> > +++ b/src/test/test_driver.c
> > @@ -26,8 +26,6 @@
> >  #include 
> >  #include 
> >  #include 
> > -
> > -
> 
> Unrelated whitespace change.

Ack

> 
> >  #include "virerror.h"
> >  #include "datatypes.h"
> >  #include "test_driver.h"
> > @@ -10035,6 +10033,62 @@
> testConnectGetDomainCapabilities(virConnectPtr conn G_GNUC_UNUSED,
> >  return virDomainCapsFormat(domCaps);
> >  }
> >
> > +static int
> > +testVirDomainAttachDeviceFlags(virDomainPtr domain,
> > +   const char *xml,
> > +   unsigned int flags G_GNUC_UNUSED) {
> > +
> > +int ret = -1;
> > +virDomainObj *vm;
> > +testDriver *driver;
> > +virDomainDeviceDef *devConf;
> > +
> > +if (!(vm = testDomObjFromDomain(domain)))
> > +return -1;
> > +
> > +driver = domain->conn->privateData;
> > +
> > +if (!(devConf = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt,
> > +NULL, 0)))
> > +goto out;
> > +
> > +VIR_APPEND_ELEMENT(vm->def->hostdevs, vm->def->nhostdevs,
> > +devConf->data.hostdev);
> 
> Aling 'devConf' with the '(' above it

Ack

> 
> > +virDomainDeviceDefFree(devConf);
> > +ret = 0;
> > +out:
> > +virDomainObjEndAPI(&vm);
> > +return ret;
> > +}
> > +
> > +static int
> > +testVirDomainAttachDevice(virDomainPtr domain, const char *xml)
> > +{
> > +return testVirDomainAttachDeviceFlags(domain, xml, 0);
> > +}
> > +
> > +static int
> > +testVirDomainDetachDeviceAlias(virDomainPtr domain,
> > +   const char *alias,
> > +   unsigned int flags __attribute__((unused)))
> 
> Horizontal alignemtn is off, and also needs to be G_GNUC_UNUSED
> like the earlier method.

Ack

> 
> > +{
> > +virDomainObj *vm;
> > +int i;
> 
> size_t for loop iterators please.

Ack

> 
> > +bool found = false;
> > +
> > +if (!(vm = testDomObjFromDomain(domain)))
> > +return -1;
> > +
> > +for (i = 0; i < vm->def->nhostdevs && !found; i++) {
> > +if (!strcmp(vm->def->hostdevs[i]->info->alias, alias)) {
> > +virDomainHostdevDefFree(vm->def->hostdevs[i]);
> > +VIR_DELETE_ELEMENT(vm->def->hostdevs, i, vm->def->nhostdevs);
> > +found = true;
> > +}
> > +}
> > +virDomainObjEndAPI(&vm);
> > +return found ? 0 : -1;
> > +}
> >
> >  /*
> >   * Test driver
> > @@ -10058,6 +10112,9 @@ static virHypervisorDriver testHypervisorDriver =
> {
> >  .connectListDomains = testConnectListDomains, /* 0.1.1 */
> >  .connectNumOfDomains = testConnectNumOfDomains, /* 0.1.1 */
> >  .connectListAllDomains = testConnectListAllDomains, /* 0.9.13 */
> > +.domainAttachDevice = testVirDomainAttachDevice, /* 9.9.0 */
> > +.domainAttachDeviceFlags = testVirDomainAttachDeviceFlags, /* 9.9.0 */
> > +.domainDetachDeviceAlias = testVirDomainDetachDeviceAlias, /* 9.9.0 */
> >  .domainCreateXML = testDomainCreateXML, /* 0.1.4 */
> >  .domainCreateXMLWithFiles = testDomainCreateXMLWithFiles, /* 5.7.0 */
> >  .domainLookupByID = testDomainLookupByID, /* 0.1.1 */
> > --
> > 2.27.0
> >
> 
> With regards,
> Daniel
> --
> |: https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__berrange.com&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=XTpYsh5Ps2zJ
> vtw6ogtti46atk736SI4vgsJiUKIyDE&m=7tOGBG0mrTLgztzu23pWZjCQpfdKbv7cV
> ilnYobvnzJSHk-
> m3fVMonE71FEjfJUc&s=MKxz2dbrxpoqVQEKrR1rIHTDNh3TQzk6j9ILpLJO4eI&e
> =   -o-https://urld

[PATCH V2] support for hotplug/hotunplug in test hypervisor

2023-10-30 Thread Thanos Makatos 
Signed-off-by: Thanos Makatos 
---
 src/test/test_driver.c | 59 ++
 1 file changed, 59 insertions(+)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index e87d7cfd44..80ef1b3cbb 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -10035,6 +10035,62 @@ testConnectGetDomainCapabilities(virConnectPtr conn 
G_GNUC_UNUSED,
 return virDomainCapsFormat(domCaps);
 }
 
+static int
+testVirDomainAttachDeviceFlags(virDomainPtr domain,
+   const char *xml,
+   unsigned int flags G_GNUC_UNUSED) {
+
+int ret = -1;
+virDomainObj *vm;
+testDriver *driver;
+virDomainDeviceDef *devConf;
+
+if (!(vm = testDomObjFromDomain(domain)))
+return -1;
+
+driver = domain->conn->privateData;
+
+if (!(devConf = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt,
+NULL, 0)))
+goto out;
+
+VIR_APPEND_ELEMENT(vm->def->hostdevs, vm->def->nhostdevs,
+   devConf->data.hostdev);
+virDomainDeviceDefFree(devConf);
+ret = 0;
+out:
+virDomainObjEndAPI(&vm);
+return ret;
+}
+
+static int
+testVirDomainAttachDevice(virDomainPtr domain, const char *xml)
+{
+return testVirDomainAttachDeviceFlags(domain, xml, 0);
+}
+
+static int
+testVirDomainDetachDeviceAlias(virDomainPtr domain,
+   const char *alias,
+   unsigned int flags G_GNUC_UNUSED)
+{
+virDomainObj *vm;
+int size_t;
+bool found = false;
+
+if (!(vm = testDomObjFromDomain(domain)))
+return -1;
+
+for (i = 0; i < vm->def->nhostdevs && !found; i++) {
+if (!strcmp(vm->def->hostdevs[i]->info->alias, alias)) {
+virDomainHostdevDefFree(vm->def->hostdevs[i]);
+VIR_DELETE_ELEMENT(vm->def->hostdevs, i, vm->def->nhostdevs);
+found = true;
+}
+}
+virDomainObjEndAPI(&vm);
+return found ? 0 : -1;
+}
 
 /*
  * Test driver
@@ -10058,6 +10114,9 @@ static virHypervisorDriver testHypervisorDriver = {
 .connectListDomains = testConnectListDomains, /* 0.1.1 */
 .connectNumOfDomains = testConnectNumOfDomains, /* 0.1.1 */
 .connectListAllDomains = testConnectListAllDomains, /* 0.9.13 */
+.domainAttachDevice = testVirDomainAttachDevice, /* 9.9.0 */
+.domainAttachDeviceFlags = testVirDomainAttachDeviceFlags, /* 9.9.0 */
+.domainDetachDeviceAlias = testVirDomainDetachDeviceAlias, /* 9.9.0 */
 .domainCreateXML = testDomainCreateXML, /* 0.1.4 */
 .domainCreateXMLWithFiles = testDomainCreateXMLWithFiles, /* 5.7.0 */
 .domainLookupByID = testDomainLookupByID, /* 0.1.1 */
-- 
2.27.0

[PATCH V3] support for hotplug/hotunplug in test hypervisor

2023-10-30 Thread Thanos Makatos 
Signed-off-by: Thanos Makatos 
---
 src/test/test_driver.c | 59 ++
 1 file changed, 59 insertions(+)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index e87d7cfd44..d605649262 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -10035,6 +10035,62 @@ testConnectGetDomainCapabilities(virConnectPtr conn 
G_GNUC_UNUSED,
 return virDomainCapsFormat(domCaps);
 }
 
+static int
+testVirDomainAttachDeviceFlags(virDomainPtr domain,
+   const char *xml,
+   unsigned int flags G_GNUC_UNUSED) {
+
+int ret = -1;
+virDomainObj *vm;
+testDriver *driver;
+virDomainDeviceDef *devConf;
+
+if (!(vm = testDomObjFromDomain(domain)))
+return -1;
+
+driver = domain->conn->privateData;
+
+if (!(devConf = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt,
+NULL, 0)))
+goto out;
+
+VIR_APPEND_ELEMENT(vm->def->hostdevs, vm->def->nhostdevs,
+   devConf->data.hostdev);
+virDomainDeviceDefFree(devConf);
+ret = 0;
+out:
+virDomainObjEndAPI(&vm);
+return ret;
+}
+
+static int
+testVirDomainAttachDevice(virDomainPtr domain, const char *xml)
+{
+return testVirDomainAttachDeviceFlags(domain, xml, 0);
+}
+
+static int
+testVirDomainDetachDeviceAlias(virDomainPtr domain,
+   const char *alias,
+   unsigned int flags G_GNUC_UNUSED)
+{
+virDomainObj *vm;
+size_t i;
+bool found = false;
+
+if (!(vm = testDomObjFromDomain(domain)))
+return -1;
+
+for (i = 0; i < vm->def->nhostdevs && !found; i++) {
+if (!strcmp(vm->def->hostdevs[i]->info->alias, alias)) {
+virDomainHostdevDefFree(vm->def->hostdevs[i]);
+VIR_DELETE_ELEMENT(vm->def->hostdevs, i, vm->def->nhostdevs);
+found = true;
+}
+}
+virDomainObjEndAPI(&vm);
+return found ? 0 : -1;
+}
 
 /*
  * Test driver
@@ -10058,6 +10114,9 @@ static virHypervisorDriver testHypervisorDriver = {
 .connectListDomains = testConnectListDomains, /* 0.1.1 */
 .connectNumOfDomains = testConnectNumOfDomains, /* 0.1.1 */
 .connectListAllDomains = testConnectListAllDomains, /* 0.9.13 */
+.domainAttachDevice = testVirDomainAttachDevice, /* 9.9.0 */
+.domainAttachDeviceFlags = testVirDomainAttachDeviceFlags, /* 9.9.0 */
+.domainDetachDeviceAlias = testVirDomainDetachDeviceAlias, /* 9.9.0 */
 .domainCreateXML = testDomainCreateXML, /* 0.1.4 */
 .domainCreateXMLWithFiles = testDomainCreateXMLWithFiles, /* 5.7.0 */
 .domainLookupByID = testDomainLookupByID, /* 0.1.1 */
-- 
2.27.0

RE: [PATCH V2] support for hotplug/hotunplug in test hypervisor

2023-10-30 Thread Thanos Makatos 


> -Original Message-
> From: Daniel P. Berrangé 
> Sent: Monday, October 30, 2023 3:55 PM
> To: Thanos Makatos 
> Cc: devel@lists.libvirt.org
> Subject: Re: [PATCH V2] support for hotplug/hotunplug in test hypervisor
> 
> On Mon, Oct 30, 2023 at 03:16:49PM +, Thanos Makatos wrote:
> > Signed-off-by: Thanos Makatos 
> > ---
> >  src/test/test_driver.c | 59
> ++
> >  1 file changed, 59 insertions(+)
> >
> > diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> > index e87d7cfd44..80ef1b3cbb 100644
> > --- a/src/test/test_driver.c
> > +++ b/src/test/test_driver.c
> > @@ -10035,6 +10035,62 @@
> testConnectGetDomainCapabilities(virConnectPtr conn G_GNUC_UNUSED,
> >  return virDomainCapsFormat(domCaps);
> >  }
> >
> > +static int
> > +testVirDomainAttachDeviceFlags(virDomainPtr domain,
> > +   const char *xml,
> > +   unsigned int flags G_GNUC_UNUSED) {
> > +
> > +int ret = -1;
> > +virDomainObj *vm;
> > +testDriver *driver;
> > +virDomainDeviceDef *devConf;
> > +
> > +if (!(vm = testDomObjFromDomain(domain)))
> > +return -1;
> > +
> > +driver = domain->conn->privateData;
> > +
> > +if (!(devConf = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt,
> > +NULL, 0)))
> > +goto out;
> > +
> > +VIR_APPEND_ELEMENT(vm->def->hostdevs, vm->def->nhostdevs,
> > +   devConf->data.hostdev);
> > +virDomainDeviceDefFree(devConf);
> > +ret = 0;
> > +out:
> > +virDomainObjEndAPI(&vm);
> > +return ret;
> > +}
> > +
> > +static int
> > +testVirDomainAttachDevice(virDomainPtr domain, const char *xml)
> > +{
> > +return testVirDomainAttachDeviceFlags(domain, xml, 0);
> > +}
> > +
> > +static int
> > +testVirDomainDetachDeviceAlias(virDomainPtr domain,
> > +   const char *alias,
> > +   unsigned int flags G_GNUC_UNUSED)
> > +{
> > +virDomainObj *vm;
> > +int size_t;
> 
> ^ this doesn't look right and won't even compile i expect

Sorry!

> 
> > +bool found = false;
> > +
> > +if (!(vm = testDomObjFromDomain(domain)))
> > +return -1;
> > +
> > +for (i = 0; i < vm->def->nhostdevs && !found; i++) {
> > +if (!strcmp(vm->def->hostdevs[i]->info->alias, alias)) {
> > +virDomainHostdevDefFree(vm->def->hostdevs[i]);
> > +VIR_DELETE_ELEMENT(vm->def->hostdevs, i, vm->def->nhostdevs);
> > +found = true;
> > +}
> > +}
> > +virDomainObjEndAPI(&vm);
> > +return found ? 0 : -1;
> > +}
> >
> >  /*
> >   * Test driver
> > @@ -10058,6 +10114,9 @@ static virHypervisorDriver testHypervisorDriver =
> {
> >  .connectListDomains = testConnectListDomains, /* 0.1.1 */
> >  .connectNumOfDomains = testConnectNumOfDomains, /* 0.1.1 */
> >  .connectListAllDomains = testConnectListAllDomains, /* 0.9.13 */
> > +.domainAttachDevice = testVirDomainAttachDevice, /* 9.9.0 */
> > +.domainAttachDeviceFlags = testVirDomainAttachDeviceFlags, /* 9.9.0 */
> > +.domainDetachDeviceAlias = testVirDomainDetachDeviceAlias, /* 9.9.0 */
> >  .domainCreateXML = testDomainCreateXML, /* 0.1.4 */
> >  .domainCreateXMLWithFiles = testDomainCreateXMLWithFiles, /* 5.7.0 */
> >  .domainLookupByID = testDomainLookupByID, /* 0.1.1 */
> > --
> > 2.27.0
> >
> 
> With regards,
> Daniel
> --
> |: https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__berrange.com&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=XTpYsh5Ps2zJ
> vtw6ogtti46atk736SI4vgsJiUKIyDE&m=DpeOKh65K_mz6mh42NlATYaEk3ZsMQx
> 5Bno7s_D1qKjMUJkMdZromAXFGUfBlvDe&s=vvMNvtjmjnCsNFz4K2joLdIpICFXk
> T-Bq7Cjvd3F6QI&e=   -o-
> https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__www.flickr.com_photos_dberrange&d=DwIBaQ&c=s883GpUCOChKOHiocYt
> Gcg&r=XTpYsh5Ps2zJvtw6ogtti46atk736SI4vgsJiUKIyDE&m=DpeOKh65K_mz6m
> h42NlATYaEk3ZsMQx5Bno7s_D1qKjMUJkMdZromAXFGUfBlvDe&s=zxZbPx43V6
> CAvORu7PztYWUGxePHdAvhlBzJ2lTIfjc&e=  :|
> |: https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__libvirt.org&d=DwIBaQ&c=s883GpUCOChKOHiocYtGcg&r=XTpYsh5Ps2zJvtw
>

RE: [PATCH V3] support for hotplug/hotunplug in test hypervisor

2023-11-07 Thread Thanos Makatos 
> -Original Message-
> From: Michal Prívozník 
> Sent: Tuesday, October 31, 2023 9:50 AM
> To: Thanos Makatos ; devel@lists.libvirt.org
> Subject: Re: [PATCH V3] support for hotplug/hotunplug in test hypervisor
> 
> On 10/30/23 17:02, Thanos Makatos wrote:
> > Signed-off-by: Thanos Makatos 
> > ---
> >  src/test/test_driver.c | 59
> ++
> >  1 file changed, 59 insertions(+)
> >
> 
> We're getting there.
> 
> > diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> > index e87d7cfd44..d605649262 100644
> > --- a/src/test/test_driver.c
> > +++ b/src/test/test_driver.c
> > @@ -10035,6 +10035,62 @@
> testConnectGetDomainCapabilities(virConnectPtr conn G_GNUC_UNUSED,
> >  return virDomainCapsFormat(domCaps);
> >  }
> >
> > +static int
> > +testVirDomainAttachDeviceFlags(virDomainPtr domain,
> 
> We construct names a bit differently. For virDomainSomething() public
> API, respective driver implementations should be named:
> s/^vir/${driver}/ for instance qemuDomainSomething() for the qemu
> driver, testDomainSomething() for the test driver. Therefore, this
> should have been: testDomainAttachDeviceFlags().

Ack.

> 
> > +   const char *xml,
> > +   unsigned int flags G_GNUC_UNUSED) {
> 
> Nope, flags must be checked. Always. We have a handy function for that
> (virCheckFlags()) but you'll need a slightly different approach anyway.

Ack.

> 
> > +
> > +int ret = -1;
> > +virDomainObj *vm;
> > +testDriver *driver;
> > +virDomainDeviceDef *devConf;
> > +
> > +if (!(vm = testDomObjFromDomain(domain)))
> > +return -1;
> > +
> > +driver = domain->conn->privateData;
> > +
> > +if (!(devConf = virDomainDeviceDefParse(xml, vm->def, driver->xmlopt,
> > +NULL, 0)))
> > +goto out;
> > +
> > +VIR_APPEND_ELEMENT(vm->def->hostdevs, vm->def->nhostdevs,
> > +   devConf->data.hostdev);
> > +virDomainDeviceDefFree(devConf);
> 
> This can't work. There are plenty of more devices that can be hotplugged
> than just . In fact, if an  was on the input, then
> devConf->data.hostdev points to some garbage.
> 
> But anyway, I think we want more elaborate approach. The whole point of
> the test driver is to mimic real guest(s) without resources needed. IOW,
> it's just a stub that developers can use to write/test their code
> against and as such it should mimic real world as close as
> possible/practical. For instance, in real world this API accepts
> basically two flags: VIR_DOMAIN_AFFECT_LIVE and
> VIR_DOMAIN_AFFECT_CONFIG
> which can be specified at the same time or independently of each other.
> But this code does not account for that.

We care about VIR_DOMAIN_AFFECT_LIVE so I'll support only this, for now.

> 
> What you can do is take inspiration in some real world driver, copy it
> over to test driver and then remove actual talking to hypervisor. For
> instance, you can see how attach is done in QEMU driver (which is our
> most mature driver and thus code there is usually maintained the best),
> keep all vir...() calls and remove qemu...() calls, basically.

I followed your suggestion and ended up with similar but substantially smaller 
code as I had to omit any code that eventually looks at the actual system (e.g. 
checking that the device sysfs entry exists, setting up cgroups, etc.).

One thing I didn't clarify in the original patch is that we don’t care whether 
the physical device exists, we just want the test hypervisor to pretend it 
does. I suppose we can support this by manually creating a mock sysfs directory 
which the test hypervisor can use, however such functionality isn't necessary 
for our immediate needs so I'd be inclined to skip it for now.

> 
> Oh, and do not forget to emit an event after successful hotplug. It
> contributes to "real world feeling".

Are you referring to virDomainAuditHostdev?

> 
> > +ret = 0;
> > +out:
> > +virDomainObjEndAPI(&vm);
> > +return ret;
> > +}
> > +
> > +static int
> > +testVirDomainAttachDevice(virDomainPtr domain, const char *xml)
> > +{
> > +return testVirDomainAttachDeviceFlags(domain, xml, 0);
> > +}
> > +
> > +static int
> > +testVirDomainDetachDeviceAlias(virDomainPtr domain,
> > +   const char *alias,
> > +   unsigned int flags G_GNUC_UNUSED)

[PATCH V4] support for hotplug/hotunplug in test hypervisor

2023-11-09 Thread Thanos Makatos 
Signed-off-by: Thanos Makatos 
---
 src/test/test_driver.c | 387 -
 1 file changed, 385 insertions(+), 2 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index e87d7cfd44..a3b4799a0f 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -26,8 +26,6 @@
 #include 
 #include 
 #include 
-
-
 #include "virerror.h"
 #include "datatypes.h"
 #include "test_driver.h"
@@ -38,10 +36,14 @@
 #include "virnetworkobj.h"
 #include "interface_conf.h"
 #include "checkpoint_conf.h"
+#include "domain_addr.h"
+#include "domain_audit.h"
 #include "domain_conf.h"
 #include "domain_driver.h"
 #include "domain_event.h"
+#include "domain_validate.h"
 #include "network_event.h"
+#include "nwfilter_conf.h"
 #include "snapshot_conf.h"
 #include "virfdstream.h"
 #include "storage_conf.h"
@@ -50,6 +52,7 @@
 #include "node_device_conf.h"
 #include "virnodedeviceobj.h"
 #include "node_device_event.h"
+#include "vircgroup.h"
 #include "virxml.h"
 #include "virthread.h"
 #include "virlog.h"
@@ -10035,6 +10038,383 @@ testConnectGetDomainCapabilities(virConnectPtr conn 
G_GNUC_UNUSED,
 return virDomainCapsFormat(domCaps);
 }
 
+static int
+testDomainAttachHostPCIDevice(testDriver *driver G_GNUC_UNUSED,
+  virDomainObj *vm,
+  virDomainHostdevDef *hostdev)
+{
+int backend = hostdev->source.subsys.u.pci.backend;
+
+switch ((virDomainHostdevSubsysPCIBackendType)backend) {
+case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO:
+case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT:
+case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM:
+break;
+
+case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN:
+case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("test hypervisor does not support device assignment 
mode '%s'"),
+   virDomainHostdevSubsysPCIBackendTypeToString(backend));
+return -1;
+}
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("guest unexpectedly quit during hotplug"));
+return -1;
+}
+
+VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1);
+
+virDomainAuditHostdev(vm, hostdev, "attach", true);
+
+vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
+
+return 0;
+}
+
+static int
+testDomainAttachHostDevice(testDriver *driver,
+   virDomainObj *vm,
+   virDomainHostdevDef *hostdev)
+{
+if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("hotplug is not supported for hostdev mode '%s'"),
+   virDomainHostdevModeTypeToString(hostdev->mode));
+return -1;
+}
+
+if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("hotplug is not supported for hostdev subsys type 
'%s'"),
+   
virDomainHostdevSubsysTypeToString(hostdev->source.subsys.type));
+return -1;
+}
+
+return testDomainAttachHostPCIDevice(driver, vm, hostdev);
+}
+
+static int
+testDomainAttachDeviceLive(virDomainObj *vm,
+   virDomainDeviceDef *dev,
+   testDriver *driver)
+{
+int ret = -1;
+const char *alias = NULL;
+
+if ((virDomainDeviceType)dev->type != VIR_DOMAIN_DEVICE_HOSTDEV) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+   _("live attach of device '%s' is not supported"),
+   virDomainDeviceTypeToString(dev->type));
+return -1;
+}
+
+testDomainObjCheckHostdevTaint(vm, dev->data.hostdev);
+if ((ret = testDomainAttachHostDevice(driver, vm, dev->data.hostdev)) != 0)
+return ret;
+
+alias = dev->data.hostdev->info->alias;
+dev->data.hostdev = NULL;
+
+if (alias) {
+virObjectEvent *event;
+event = virDomainEventDeviceAddedNewFromObj(vm, alias);
+virObjectEventStateQueue(driver->eventState, event);
+}
+
+return 0;
+}
+
+static int
+testDomainAttachDeviceLiveAndConfig(virDomainObj *vm,
+testDriver *driver,
+const char *xml,
+unsigned int flags)
+{
+g_autoptr(virDomainDeviceDef) devConf = NULL;
+g_autoptr(virDomainDeviceDef) devLive = NULL;
+

[PATCH V5] support for hotplug/hotunplug in test hypervisor

2023-11-09 Thread Thanos Makatos 
Signed-off-by: Thanos Makatos 

---

Changed since v4:
* removed inadvertent calls to functions 
virNWFilterReadLockFilterUpdates/virNWFilterUnlockFilterUpdates
  (original patch was based on v8.0.0)

---
 src/test/test_driver.c | 382 -
 1 file changed, 380 insertions(+), 2 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index e87d7cfd44..effb7f9880 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -26,8 +26,6 @@
 #include 
 #include 
 #include 
-
-
 #include "virerror.h"
 #include "datatypes.h"
 #include "test_driver.h"
@@ -38,9 +36,12 @@
 #include "virnetworkobj.h"
 #include "interface_conf.h"
 #include "checkpoint_conf.h"
+#include "domain_addr.h"
+#include "domain_audit.h"
 #include "domain_conf.h"
 #include "domain_driver.h"
 #include "domain_event.h"
+#include "domain_validate.h"
 #include "network_event.h"
 #include "snapshot_conf.h"
 #include "virfdstream.h"
@@ -50,6 +51,7 @@
 #include "node_device_conf.h"
 #include "virnodedeviceobj.h"
 #include "node_device_event.h"
+#include "vircgroup.h"
 #include "virxml.h"
 #include "virthread.h"
 #include "virlog.h"
@@ -10035,6 +10037,379 @@ testConnectGetDomainCapabilities(virConnectPtr conn 
G_GNUC_UNUSED,
 return virDomainCapsFormat(domCaps);
 }
 
+static int
+testDomainAttachHostPCIDevice(testDriver *driver G_GNUC_UNUSED,
+  virDomainObj *vm,
+  virDomainHostdevDef *hostdev)
+{
+int backend = hostdev->source.subsys.u.pci.backend;
+
+switch ((virDomainHostdevSubsysPCIBackendType)backend) {
+case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO:
+case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT:
+case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM:
+break;
+
+case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN:
+case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("test hypervisor does not support device assignment 
mode '%s'"),
+   virDomainHostdevSubsysPCIBackendTypeToString(backend));
+return -1;
+}
+
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("guest unexpectedly quit during hotplug"));
+return -1;
+}
+
+VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1);
+
+virDomainAuditHostdev(vm, hostdev, "attach", true);
+
+vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
+
+return 0;
+}
+
+static int
+testDomainAttachHostDevice(testDriver *driver,
+   virDomainObj *vm,
+   virDomainHostdevDef *hostdev)
+{
+if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("hotplug is not supported for hostdev mode '%s'"),
+   virDomainHostdevModeTypeToString(hostdev->mode));
+return -1;
+}
+
+if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("hotplug is not supported for hostdev subsys type 
'%s'"),
+   
virDomainHostdevSubsysTypeToString(hostdev->source.subsys.type));
+return -1;
+}
+
+return testDomainAttachHostPCIDevice(driver, vm, hostdev);
+}
+
+static int
+testDomainAttachDeviceLive(virDomainObj *vm,
+   virDomainDeviceDef *dev,
+   testDriver *driver)
+{
+int ret = -1;
+const char *alias = NULL;
+
+if ((virDomainDeviceType)dev->type != VIR_DOMAIN_DEVICE_HOSTDEV) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+   _("live attach of device '%s' is not supported"),
+   virDomainDeviceTypeToString(dev->type));
+return -1;
+}
+
+testDomainObjCheckHostdevTaint(vm, dev->data.hostdev);
+if ((ret = testDomainAttachHostDevice(driver, vm, dev->data.hostdev)) != 0)
+return ret;
+
+alias = dev->data.hostdev->info->alias;
+dev->data.hostdev = NULL;
+
+if (alias) {
+virObjectEvent *event;
+event = virDomainEventDeviceAddedNewFromObj(vm, alias);
+virObjectEventStateQueue(driver->eventState, event);
+}
+
+return 0;
+}
+
+static int
+testDomainAttachDeviceLiveAndConfig(virDomainObj *vm,
+testDriver *driver,
+const char *xml,
+unsigned int f

RE: [PATCH V5] support for hotplug/hotunplug in test hypervisor

2023-11-22 Thread Thanos Makatos 
ping

> -Original Message-
> From: Thanos Makatos 
> Sent: Thursday, November 9, 2023 11:15 AM
> To: devel@lists.libvirt.org
> Cc: Thanos Makatos 
> Subject: [PATCH V5] support for hotplug/hotunplug in test hypervisor
> 
> Signed-off-by: Thanos Makatos 
> 
> ---
> 
> Changed since v4:
> * removed inadvertent calls to functions
> virNWFilterReadLockFilterUpdates/virNWFilterUnlockFilterUpdates
>   (original patch was based on v8.0.0)
> 
> ---
>  src/test/test_driver.c | 382 -
>  1 file changed, 380 insertions(+), 2 deletions(-)
> 
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index e87d7cfd44..effb7f9880 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -26,8 +26,6 @@
>  #include 
>  #include 
>  #include 
> -
> -
>  #include "virerror.h"
>  #include "datatypes.h"
>  #include "test_driver.h"
> @@ -38,9 +36,12 @@
>  #include "virnetworkobj.h"
>  #include "interface_conf.h"
>  #include "checkpoint_conf.h"
> +#include "domain_addr.h"
> +#include "domain_audit.h"
>  #include "domain_conf.h"
>  #include "domain_driver.h"
>  #include "domain_event.h"
> +#include "domain_validate.h"
>  #include "network_event.h"
>  #include "snapshot_conf.h"
>  #include "virfdstream.h"
> @@ -50,6 +51,7 @@
>  #include "node_device_conf.h"
>  #include "virnodedeviceobj.h"
>  #include "node_device_event.h"
> +#include "vircgroup.h"
>  #include "virxml.h"
>  #include "virthread.h"
>  #include "virlog.h"
> @@ -10035,6 +10037,379 @@
> testConnectGetDomainCapabilities(virConnectPtr conn G_GNUC_UNUSED,
>  return virDomainCapsFormat(domCaps);
>  }
> 
> +static int
> +testDomainAttachHostPCIDevice(testDriver *driver G_GNUC_UNUSED,
> +  virDomainObj *vm,
> +  virDomainHostdevDef *hostdev)
> +{
> +int backend = hostdev->source.subsys.u.pci.backend;
> +
> +switch ((virDomainHostdevSubsysPCIBackendType)backend) {
> +case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO:
> +case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT:
> +case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM:
> +break;
> +
> +case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN:
> +case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST:
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("test hypervisor does not support device assignment 
> mode
> '%s'"),
> +   
> virDomainHostdevSubsysPCIBackendTypeToString(backend));
> +return -1;
> +}
> +
> +if (!virDomainObjIsActive(vm)) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("guest unexpectedly quit during hotplug"));
> +return -1;
> +}
> +
> +VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1);
> +
> +virDomainAuditHostdev(vm, hostdev, "attach", true);
> +
> +vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
> +
> +return 0;
> +}
> +
> +static int
> +testDomainAttachHostDevice(testDriver *driver,
> +   virDomainObj *vm,
> +   virDomainHostdevDef *hostdev)
> +{
> +if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("hotplug is not supported for hostdev mode '%s'"),
> +   virDomainHostdevModeTypeToString(hostdev->mode));
> +return -1;
> +}
> +
> +if (hostdev->source.subsys.type !=
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("hotplug is not supported for hostdev subsys type 
> '%s'"),
> +   virDomainHostdevSubsysTypeToString(hostdev-
> >source.subsys.type));
> +return -1;
> +}
> +
> +return testDomainAttachHostPCIDevice(driver, vm, hostdev);
> +}
> +
> +static int
> +testDomainAttachDeviceLive(virDomainObj *vm,
> +   virDomainDeviceDef *dev,
> +   testDriver *driver)
> +{
> +int ret = -1;
> +const char *alias = NULL;
> +
> +if ((virDomainDeviceType)dev->type != VIR_DOMAIN_DEVICE_HOSTDEV) {
> +virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> +   _("

RE: [PATCH V5] support for hotplug/hotunplug in test hypervisor

2023-11-29 Thread Thanos Makatos 
> -Original Message-
> From: Michal Prívozník 
> Sent: Tuesday, November 28, 2023 1:19 PM
> To: Thanos Makatos ; devel@lists.libvirt.org
> Subject: Re: [PATCH V5] support for hotplug/hotunplug in test hypervisor
> 
> On 11/9/23 12:14, Thanos Makatos wrote:
> > Signed-off-by: Thanos Makatos 
> >
> > ---
> >
> > Changed since v4:
> > * removed inadvertent calls to functions
> virNWFilterReadLockFilterUpdates/virNWFilterUnlockFilterUpdates
> >   (original patch was based on v8.0.0)
> >
> > ---
> >  src/test/test_driver.c | 382
> -
> >  1 file changed, 380 insertions(+), 2 deletions(-)
> 
> Ooops, this has missed my attention and unfortunately upcoming release
> too as we are in freeze. Sorry. To make it up to you, I'm fixing all the
> small issues I'm raising below and will push once after the release.
> 
> I've uploaded the fixup patch (that I'll squash into yours before
> merging) here, if you want to take a look:
> 
> https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__gitlab.com_MichalPrivoznik_libvirt_-2D_commits_test-5Fdriver-
> 5Fhotplug_-3Fref-5Ftype-
> 3Dheads&d=DwICaQ&c=s883GpUCOChKOHiocYtGcg&r=XTpYsh5Ps2zJvtw6ogtti
> 46atk736SI4vgsJiUKIyDE&m=o5_V_Q03_HbW5P4oVf5Tez4K2mYV_jf35yvol4O5
> KioGaofPI3B07QN9CvepCKAc&s=kiKbd_XHWn3_p3luPq89G0pku0iApEIs18MAtL
> u_d88&e=
> 
> >
> > diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> > index e87d7cfd44..effb7f9880 100644
> > --- a/src/test/test_driver.c
> > +++ b/src/test/test_driver.c
> > @@ -26,8 +26,6 @@
> >  #include 
> >  #include 
> >  #include 
> > -
> > -
> 
> We like to keep these separate to make it more obvious where public
> header section ends and private header section starts.
> 
> >  #include "virerror.h"
> >  #include "datatypes.h"
> >  #include "test_driver.h"
> > @@ -38,9 +36,12 @@
> >  #include "virnetworkobj.h"
> >  #include "interface_conf.h"
> >  #include "checkpoint_conf.h"
> > +#include "domain_addr.h"
> > +#include "domain_audit.h"
> 
> This shouldn't be needed. domain_audit contains functions to audit real
> resource auditing (e.g. assigning a real device to a guest) - test
> driver should not do such thing.
> 
> >  #include "domain_conf.h"
> >  #include "domain_driver.h"
> >  #include "domain_event.h"
> > +#include "domain_validate.h"
> >  #include "network_event.h"
> >  #include "snapshot_conf.h"
> >  #include "virfdstream.h"
> > @@ -50,6 +51,7 @@
> >  #include "node_device_conf.h"
> >  #include "virnodedeviceobj.h"
> >  #include "node_device_event.h"
> > +#include "vircgroup.h"
> 
> And this too shouldn't be here. We do not set up any cgroups for domains
> under the test driver.
> 
> >  #include "virxml.h"
> >  #include "virthread.h"
> >  #include "virlog.h"
> > @@ -10035,6 +10037,379 @@
> testConnectGetDomainCapabilities(virConnectPtr conn G_GNUC_UNUSED,
> >  return virDomainCapsFormat(domCaps);
> >  }
> >
> > +static int
> > +testDomainAttachHostPCIDevice(testDriver *driver G_GNUC_UNUSED,
> > +  virDomainObj *vm,
> > +  virDomainHostdevDef *hostdev)
> > +{
> > +int backend = hostdev->source.subsys.u.pci.backend;
> > +
> > +switch ((virDomainHostdevSubsysPCIBackendType)backend) {
> > +case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO:
> > +case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT:
> > +case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM:
> > +break;
> > +
> > +case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN:
> > +case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST:
> > +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > +   _("test hypervisor does not support device 
> > assignment mode
> '%s'"),
> 
> I'll raise it here: for translatable strings, we've switched to using
> more robust format: %1$s because it bit us in the past when one of
> locales contained wrong data causing us to crash. This should have been
> caught by 'ninja test'.
> 
> > +   
> > virDomainHostdevSubsysPCIBackendTypeToString(backend));
> > +return -1;
> > +}
> > +
> > +if (!virDomai

join running core dump job

2024-03-04 Thread Thanos Makatos 
Is there a way to programmatically wait for a previously initiated 
virDomainCoreDumpWithFormat() where the process that started it died? I'm 
looking at the API and don't seem to find anything relevant.  I suppose I could 
poll via virDomainGetJobStats(), but, ideally, I'd like a function that would 
join the dump job and return when the dump job finishes.
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org

RE: join running core dump job

2024-03-04 Thread Thanos Makatos 
> -Original Message-
> From: Thanos Makatos 
> Sent: Monday, March 4, 2024 5:24 PM
> To: devel@lists.libvirt.org
> Subject: join running core dump job
> 
> Is there a way to programmatically wait for a previously initiated
> virDomainCoreDumpWithFormat() where the process that started it died? I'm
> looking at the API and don't seem to find anything relevant.  I suppose I 
> could
> poll via virDomainGetJobStats(), but, ideally, I'd like a function that would 
> join
> the dump job and return when the dump job finishes.
> ___
> Devel mailing list -- devel@lists.libvirt.org
> To unsubscribe send an email to devel-le...@lists.libvirt.org

I see there's qemuDumpWaitForCompletion(), looks promising.
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org

RE: join running core dump job

2024-03-08 Thread Thanos Makatos 
> -Original Message-
> From: Thanos Makatos 
> Sent: Monday, March 4, 2024 9:45 PM
> To: Thanos Makatos ; devel@lists.libvirt.org
> Subject: RE: join running core dump job
> 
> > -Original Message-
> > From: Thanos Makatos 
> > Sent: Monday, March 4, 2024 5:24 PM
> > To: devel@lists.libvirt.org
> > Subject: join running core dump job
> >
> > Is there a way to programmatically wait for a previously initiated
> > virDomainCoreDumpWithFormat() where the process that started it died?
> I'm
> > looking at the API and don't seem to find anything relevant.  I suppose I
> could
> > poll via virDomainGetJobStats(), but, ideally, I'd like a function that 
> > would
> join
> > the dump job and return when the dump job finishes.
> > ___
> > Devel mailing list -- devel@lists.libvirt.org
> > To unsubscribe send an email to devel-le...@lists.libvirt.org
> 
> I see there's qemuDumpWaitForCompletion(), looks promising.

I've made some progress (added a virHypervisorDriver.domainCoreDumpWait and the 
relevant scaffolding to make 'virsh dump --wait' work), calling 
qemuDumpWaitForCompletion() is all that's needed.

However, it doesn't seem trivial to implement this in the test_driver.
First, IIUC testDomainCoreDumpWithFormat() gets an exclusive lock on the domain 
(haven't tested anything yet), so calling domainCoreDumpWait() would block for 
the wrong reason. Is making testDomainCoreDumpWithFormat() using asynchronous 
jobs internally unavoidable?
Second, I want to test behaviour in my application where (1) it calls 
domainCoreDump(), (2) crashes before domainCoreDump() finishes, (3) then my 
application starts again and looks for a pending dump job and (4) joins it 
using domainCoreDumpWait(). I can't see an easy wait of faking a dump job in 
the test_driver when it starts. How about adding persistent tasks, which I can 
pre-populate before starting my application, or fake jobs via an environment 
variable, so that when the test_driver starts it can internally continue them? 
E.g. we can specify how long to run the job for and domainCoreDumpWait() add a 
sleep for that long.

I'm open to suggestions.
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org

RE: join running core dump job

2024-03-23 Thread Thanos Makatos 
> -Original Message-
> From: Thanos Makatos 
> Sent: Friday, March 8, 2024 2:21 PM
> To: devel@lists.libvirt.org
> Cc: Michal Privoznik 
> Subject: RE: join running core dump job
> 
> > -Original Message-----
> > From: Thanos Makatos 
> > Sent: Monday, March 4, 2024 9:45 PM
> > To: Thanos Makatos ;
> devel@lists.libvirt.org
> > Subject: RE: join running core dump job
> >
> > > -Original Message-
> > > From: Thanos Makatos 
> > > Sent: Monday, March 4, 2024 5:24 PM
> > > To: devel@lists.libvirt.org
> > > Subject: join running core dump job
> > >
> > > Is there a way to programmatically wait for a previously initiated
> > > virDomainCoreDumpWithFormat() where the process that started it died?
> > I'm
> > > looking at the API and don't seem to find anything relevant.  I suppose I
> > could
> > > poll via virDomainGetJobStats(), but, ideally, I'd like a function that 
> > > would
> > join
> > > the dump job and return when the dump job finishes.
> > > ___
> > > Devel mailing list -- devel@lists.libvirt.org
> > > To unsubscribe send an email to devel-le...@lists.libvirt.org
> >
> > I see there's qemuDumpWaitForCompletion(), looks promising.
> 
> I've made some progress (added a
> virHypervisorDriver.domainCoreDumpWait and the relevant scaffolding to
> make 'virsh dump --wait' work), calling qemuDumpWaitForCompletion() is all
> that's needed.
> 
> However, it doesn't seem trivial to implement this in the test_driver.
> First, IIUC testDomainCoreDumpWithFormat() gets an exclusive lock on the
> domain (haven't tested anything yet), so calling domainCoreDumpWait()
> would block for the wrong reason. Is making
> testDomainCoreDumpWithFormat() using asynchronous jobs internally
> unavoidable?
> Second, I want to test behaviour in my application where (1) it calls
> domainCoreDump(), (2) crashes before domainCoreDump() finishes, (3) then
> my application starts again and looks for a pending dump job and (4) joins it
> using domainCoreDumpWait(). I can't see an easy wait of faking a dump job
> in the test_driver when it starts. How about adding persistent tasks, which I
> can pre-populate before starting my application, or fake jobs via an
> environment variable, so that when the test_driver starts it can internally
> continue them? E.g. we can specify how long to run the job for and
> domainCoreDumpWait() add a sleep for that long.

I ended up using and environment variable in the test_driver to fake jobs, so 
the application under test doesn't need to know anything. Would something like 
that be accepted?

> 
> I'm open to suggestions.

I have managed to implement a PoC by introducing virDomainJobWait(), which 
checks the job type and if it's a dump it calls qemuDumpWaitForCompletion(), 
purely because it's the easiest thing to do (it fails with 
VIR_ERR_OPERATION_INVALID for all other operations). We'd like to implement 
this for all other job types and I'm looking for some pointers, is 
virCondWaitUntil(&priv->job.asyncCond, &obj->parent.lock) all that's required? 
(And return priv->job.error?)

Also, as I explained earlier, I want to implement joining a potentially 
existing dump job. In my specific use case, I want to either join an ongoing 
dump job if it exists, otherwise start a new one. I do this by calling 
virDomainGetJobStats(), but I'm thinking whether we could add a new, optional 
dump flag, e.g. 'join' which would make virDomainCoreDumpWithFormat do all this 
internally. Would something like that be accepted upstream. 
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org

introduce virDomainJobWait for dump plus testing

2024-04-26 Thread Thanos Makatos 
This patch series introduces funcionality for waiting for a job from a
different context, for now only for dump jobs, plus testing.

This can be useful in situations where the original requester of the job
crashes/restarts and then needs to continue waiting for that potentially
ongoing job. To avoid waiting for the wrong job type, the caller must
specify the required job type. For now I've only implemented this for
dump jobs; if the logic is correct I can extended it for all other job
types.

The first patch introduces the core functionality. I see that there is a
function specifically for waiting for a dump job:
qemuDumpWaitForCompletion.  Ideally we should only use
qemuDomainObjWait, which would work for any job type, however I'm not
sure it's correct to use it instead of qemuDumpWaitForCompletion because
the latter checks dumpCompleted. It's not obvious to me what is the
purpose of dumpCompleted, is it because qemuDomainObjWait can return
because some other job, running in parallel with the dump job, has
completed, so it's effectively a false alarm?

The second patch is mainly for testing the wait functionality via virsh.

The remaining patches extend the test hypervisor's functionality on dump
testing.

___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org

[RFC 1/6] introduce virDomainJobWait

2024-04-26 Thread Thanos Makatos 
This patch introduces virDomainJobWait, which waits for the specified
domain job to finish. The caller specifies the desired job type to wait
for. For now, only dump jobs are supported.

This functionality is needed so that the process that started the dump
job can pick it up and continue waiting for it in case it restarted.

Signed-off-by: Thanos Makatos 
---
 include/libvirt/libvirt-domain.h | 54 ++--
 src/driver-hypervisor.h  |  4 +++
 src/libvirt-domain.c | 22 -
 src/libvirt_public.syms  |  1 +
 src/qemu/qemu_driver.c   | 28 +
 src/remote/remote_driver.c   |  1 +
 src/remote/remote_protocol.x | 14 -
 src/remote_protocol-structs  |  5 +++
 8 files changed, 104 insertions(+), 25 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 30cce85b29..7ca77f2e41 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -1681,6 +1681,7 @@ int virDomainCoreDumpWithFormat 
(virDomainPtr domain,
  unsigned int dumpformat,
  unsigned int flags);
 
+
 /*
  * Screenshot of current domain console
  */
@@ -3130,6 +3131,36 @@ typedef enum {
 int virDomainBlockJobAbort(virDomainPtr dom, const char *disk,
unsigned int flags);
 
+/**
+ * virDomainJobOperation:
+ *
+ * Since: 3.3.0
+ */
+typedef enum {
+VIR_DOMAIN_JOB_OPERATION_UNKNOWN = 0, /* (Since: 3.3.0) */
+VIR_DOMAIN_JOB_OPERATION_START = 1, /* (Since: 3.3.0) */
+VIR_DOMAIN_JOB_OPERATION_SAVE = 2, /* (Since: 3.3.0) */
+VIR_DOMAIN_JOB_OPERATION_RESTORE = 3, /* (Since: 3.3.0) */
+VIR_DOMAIN_JOB_OPERATION_MIGRATION_IN = 4, /* (Since: 3.3.0) */
+VIR_DOMAIN_JOB_OPERATION_MIGRATION_OUT = 5, /* (Since: 3.3.0) */
+VIR_DOMAIN_JOB_OPERATION_SNAPSHOT = 6, /* (Since: 3.3.0) */
+VIR_DOMAIN_JOB_OPERATION_SNAPSHOT_REVERT = 7, /* (Since: 3.3.0) */
+VIR_DOMAIN_JOB_OPERATION_DUMP = 8, /* (Since: 3.3.0) */
+VIR_DOMAIN_JOB_OPERATION_BACKUP = 9, /* (Since: 6.0.0) */
+VIR_DOMAIN_JOB_OPERATION_SNAPSHOT_DELETE = 10, /* (Since: 9.0.0) */
+
+# ifdef VIR_ENUM_SENTINELS
+VIR_DOMAIN_JOB_OPERATION_LAST /* (Since: 3.3.0) */
+# endif
+} virDomainJobOperation;
+
+/*
+ * Wait for job to finish
+ *
+ * Since 10.1.0
+ */
+int virDomainJobWait (virDomainPtr domain, int op);
+
 /**
  * virDomainBlockJobInfoFlags:
  *
@@ -4185,29 +4216,6 @@ typedef enum {
 int virDomainAbortJobFlags(virDomainPtr dom,
unsigned int flags);
 
-/**
- * virDomainJobOperation:
- *
- * Since: 3.3.0
- */
-typedef enum {
-VIR_DOMAIN_JOB_OPERATION_UNKNOWN = 0, /* (Since: 3.3.0) */
-VIR_DOMAIN_JOB_OPERATION_START = 1, /* (Since: 3.3.0) */
-VIR_DOMAIN_JOB_OPERATION_SAVE = 2, /* (Since: 3.3.0) */
-VIR_DOMAIN_JOB_OPERATION_RESTORE = 3, /* (Since: 3.3.0) */
-VIR_DOMAIN_JOB_OPERATION_MIGRATION_IN = 4, /* (Since: 3.3.0) */
-VIR_DOMAIN_JOB_OPERATION_MIGRATION_OUT = 5, /* (Since: 3.3.0) */
-VIR_DOMAIN_JOB_OPERATION_SNAPSHOT = 6, /* (Since: 3.3.0) */
-VIR_DOMAIN_JOB_OPERATION_SNAPSHOT_REVERT = 7, /* (Since: 3.3.0) */
-VIR_DOMAIN_JOB_OPERATION_DUMP = 8, /* (Since: 3.3.0) */
-VIR_DOMAIN_JOB_OPERATION_BACKUP = 9, /* (Since: 6.0.0) */
-VIR_DOMAIN_JOB_OPERATION_SNAPSHOT_DELETE = 10, /* (Since: 9.0.0) */
-
-# ifdef VIR_ENUM_SENTINELS
-VIR_DOMAIN_JOB_OPERATION_LAST /* (Since: 3.3.0) */
-# endif
-} virDomainJobOperation;
-
 /**
  * VIR_DOMAIN_JOB_OPERATION:
  *
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index 5219344b72..717a5d511f 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -750,6 +750,9 @@ typedef int
 (*virDrvDomainAbortJobFlags)(virDomainPtr domain,
  unsigned int flags);
 
+typedef int
+(*virDrvDomainJobWait)(virDomainPtr domain, int op);
+
 typedef int
 (*virDrvDomainMigrateGetMaxDowntime)(virDomainPtr domain,
  unsigned long long *downtime,
@@ -1602,6 +1605,7 @@ struct _virHypervisorDriver {
 virDrvDomainGetJobStats domainGetJobStats;
 virDrvDomainAbortJob domainAbortJob;
 virDrvDomainAbortJobFlags domainAbortJobFlags;
+virDrvDomainJobWait domainJobWait;
 virDrvDomainMigrateGetMaxDowntime domainMigrateGetMaxDowntime;
 virDrvDomainMigrateSetMaxDowntime domainMigrateSetMaxDowntime;
 virDrvDomainMigrateGetCompressionCache domainMigrateGetCompressionCache;
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 83abad251e..846d711f1b 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -1527,7 +1527,6 @@ virDomainCoreDumpWithFormat(virDomainPtr domain, const 
char *to,
 return -1;
 }
 
-
 /**
  * virDomainScreenshot:
  * @domain: a domain object
@@ -10660,6 +10659,27 @@ virDomainBlockJobAbort(virDomainPtr dom, const char 
*disk

[RFC 3/6] test_driver: allow win-dmp format

2024-04-26 Thread Thanos Makatos 
Signed-off-by: Thanos Makatos 
---
 src/test/test_driver.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index ed0cdc0dab..c892ecefaa 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2621,7 +2621,8 @@ static int testDomainCoreDumpWithFormat(virDomainPtr 
domain,
 }
 
 /* we don't support non-raw formats in test driver */
-if (dumpformat != VIR_DOMAIN_CORE_DUMP_FORMAT_RAW) {
+if (dumpformat != VIR_DOMAIN_CORE_DUMP_FORMAT_RAW
+&& dumpformat != VIR_DOMAIN_CORE_DUMP_FORMAT_WIN_DMP) {
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("kdump-compressed format is not supported here"));
 goto cleanup;
-- 
2.30.2
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org

[RFC 2/6] add wait option to virsh dump

2024-04-26 Thread Thanos Makatos 
This patch introduces the --wait option to virsh dump, which allows
waiting for an already ongoing dump operation.
---
 tools/virsh-domain.c | 30 --
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index e69d14a6aa..bcaa4e7101 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -5421,7 +5421,6 @@ static const vshCmdOptDef opts_dump[] = {
 VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
 {.name = "file",
  .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
  .help = N_("where to dump the core")
 },
 VIRSH_COMMON_OPT_LIVE(N_("perform a live core dump if supported")),
@@ -5451,6 +5450,10 @@ static const vshCmdOptDef opts_dump[] = {
  .completer = virshDomainCoreDumpFormatCompleter,
  .help = N_("specify the format of memory-only dump")
 },
+{.name = "wait",
+ .type = VSH_OT_BOOL,
+ .help = N_("specify the format of memory-only dump")
+},
 {.name = NULL}
 };
 
@@ -5515,16 +5518,31 @@ doDump(void *opaque)
 }
 }
 
-if (dumpformat != VIR_DOMAIN_CORE_DUMP_FORMAT_RAW) {
-if (virDomainCoreDumpWithFormat(dom, to, dumpformat, flags) < 0) {
-vshError(ctl, _("Failed to core dump domain '%1$s' to %2$s"), 
name, to);
+if (vshCommandOptBool(cmd, "wait")) {
+if (flags != 0) {
+vshError(ctl, "%s", _("--wait cannot be used with any other 
option"));
+goto out;
+}
+if (virDomainJobWait(dom, (int)VIR_DOMAIN_JOB_OPERATION_DUMP) < 0) {
+vshError(ctl, _("Failed to wait for core dump domain"));
 goto out;
 }
 } else {
-if (virDomainCoreDump(dom, to, flags) < 0) {
-vshError(ctl, _("Failed to core dump domain '%1$s' to %2$s"), 
name, to);
+if (to == NULL) {
+vshError(ctl, _("missing required argument --file"));
 goto out;
 }
+if (dumpformat != VIR_DOMAIN_CORE_DUMP_FORMAT_RAW) {
+if (virDomainCoreDumpWithFormat(dom, to, dumpformat, flags) < 0) {
+vshError(ctl, _("Failed to core dump domain '%1$s' to %2$s"), 
name, to);
+goto out;
+}
+} else {
+if (virDomainCoreDump(dom, to, flags) < 0) {
+vshError(ctl, _("Failed to core dump domain '%1$s' to %2$s"), 
name, to);
+goto out;
+}
+}
 }
 
 data->ret = 0;
-- 
2.30.2
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org

[RFC 4/6] test_driver: introduce testDomainJobWait

2024-04-26 Thread Thanos Makatos 
This patch adds function testDomainJobWait. Since the test_driver
exists in the application's address space, it is impossible to test the
scenario where an application starts a core dump job, restarts, and then
joins it. To get around this problem, we simulate a dump job by having
testDomainGetJobStats check for the LIBVIRT_TEST_DRIVER_DUMP_JOB
environment variable and it's value used by testDomainJobWait as
the return value of the dump job.

Signed-off-by: Thanos Makatos 
---
 src/test/test_driver.c | 114 +
 1 file changed, 114 insertions(+)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index c892ecefaa..a9c7dbaee1 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1603,6 +1603,78 @@ testConnectBaselineCPU(virConnectPtr conn G_GNUC_UNUSED,
 return cpustr;
 }
 
+
+static int
+testDomainDumpJobInfoToParams(int *type,
+  virTypedParameterPtr *params,
+  int *nparams)
+{
+virTypedParameterPtr par = NULL;
+int maxpar = 0;
+int npar = 0;
+
+if (virTypedParamsAddInt(&par, &npar, &maxpar,
+ VIR_DOMAIN_JOB_OPERATION,
+ VIR_DOMAIN_JOB_OPERATION_DUMP) < 0)
+goto error;
+
+if (virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_TIME_ELAPSED,
+0) < 0)
+goto error;
+
+if (virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_MEMORY_TOTAL,
+0) < 0 ||
+virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_MEMORY_PROCESSED,
+0) < 0 ||
+virTypedParamsAddULLong(&par, &npar, &maxpar,
+VIR_DOMAIN_JOB_MEMORY_REMAINING,
+0) < 0)
+goto error;
+
+*type = VIR_DOMAIN_JOB_UNBOUNDED;
+*params = par;
+*nparams = npar;
+return 0;
+
+error:
+virTypedParamsFree(par, npar);
+return -1;
+}
+
+
+/*
+ * If the environment variable LIBVIRT_TEST_DRIVER_DUMP_JOB is set, it returns
+ * a fake dump job.
+ */
+static int
+testDomainGetJobStats(virDomainPtr dom,
+  int *type,
+  virTypedParameterPtr *params,
+  int *nparams,
+  unsigned int flags)
+{
+virDomainObj *obj;
+int ret = 0;
+
+virCheckFlags(0, -1);
+
+if (!(obj = testDomObjFromDomain(dom)))
+return -1;
+
+if (getenv("LIBVIRT_TEST_DRIVER_DUMP_JOB")) {
+VIR_DEBUG("pretending there's a dump job");
+ret = testDomainDumpJobInfoToParams(type, params, nparams);
+} else
+*type = VIR_DOMAIN_JOB_NONE;
+
+virDomainObjEndAPI(&obj);
+return ret;
+}
+
+
 static int testNodeGetInfo(virConnectPtr conn,
virNodeInfoPtr info)
 {
@@ -2656,6 +2728,46 @@ testDomainCoreDump(virDomainPtr domain,
 }
 
 
+/*
+ * If the environment variable LIBVIRT_TEST_DRIVER_DUMP_JOB is set, its value
+ * is used as the return value.
+ */
+static int
+testDomainJobWait(virDomainPtr domain, int op)
+{
+virDomainObj *vm;
+int ret = 0;
+char *s;
+
+if (!(vm = testDomObjFromDomain(domain)))
+return -1;
+
+if ((virDomainJobOperation)op != VIR_DOMAIN_JOB_OPERATION_DUMP) {
+virReportError(VIR_ERR_NO_SUPPORT,
+_("waiting for job type %d not supported "), op);
+goto out;
+}
+
+s = getenv("LIBVIRT_TEST_DRIVER_DUMP_JOB");
+if (s) {
+VIR_DEBUG("pretending there's a dump job, rc=%s", s);
+ret = atoi(s);
+if (ret != 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("dump fake fail"));
+}
+} else {
+if (virDomainObjWait(vm) < 0) {
+VIR_WARN("virDomainObjWait failed");
+ret = -1;
+}
+}
+out:
+virDomainObjEndAPI(&vm);
+return ret;
+}
+
+
 static char *
 testDomainGetOSType(virDomainPtr dom G_GNUC_UNUSED)
 {
@@ -10544,6 +10656,8 @@ static virHypervisorDriver testHypervisorDriver = {
 .domainSnapshotDelete = testDomainSnapshotDelete, /* 1.1.4 */
 
 .connectBaselineCPU = testConnectBaselineCPU, /* 1.2.0 */
+.domainGetJobStats = testDomainGetJobStats, /* 10.1.0 */
+.domainJobWait = testDomainJobWait, /* 10.1.0 */
 .domainCheckpointCreateXML = testDomainCheckpointCreateXML, /* 5.6.0 */
 .domainCheckpointGetXMLDesc = testDomainCheckpointGetXMLDesc, /* 5.6.0 */
 
-- 
2.30.2
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org

[RFC 5/6] test_driver: allow and ignore more flags in testDomainCoreDumpWithFormat

2024-04-26 Thread Thanos Makatos 
It doesn't make sense to honor them, so we may just allow them.
---
 src/test/test_driver.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index a9c7dbaee1..2a31511e3e 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2664,7 +2664,8 @@ static int testDomainCoreDumpWithFormat(virDomainPtr 
domain,
 virObjectEvent *event = NULL;
 int ret = -1;
 
-virCheckFlags(VIR_DUMP_CRASH, -1);
+virCheckFlags(VIR_DUMP_CRASH | VIR_DUMP_LIVE | VIR_DUMP_BYPASS_CACHE |
+  VIR_DUMP_MEMORY_ONLY, -1);
 
 
 if (!(privdom = testDomObjFromDomain(domain)))
-- 
2.30.2
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org

[RFC 6/6] test_driver: dump: store dump flags as payload

2024-04-26 Thread Thanos Makatos 
This way we can inspect the dump file and confirm that the test_driver
really used the dump format we specified.

Signed-off-by: Thanos Makatos 
---
 src/test/test_driver.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 2a31511e3e..4f78f39de9 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2663,6 +2663,8 @@ static int testDomainCoreDumpWithFormat(virDomainPtr 
domain,
 virDomainObj *privdom;
 virObjectEvent *event = NULL;
 int ret = -1;
+char *buf = NULL;
+int size;
 
 virCheckFlags(VIR_DUMP_CRASH | VIR_DUMP_LIVE | VIR_DUMP_BYPASS_CACHE |
   VIR_DUMP_MEMORY_ONLY, -1);
@@ -2680,7 +2682,13 @@ static int testDomainCoreDumpWithFormat(virDomainPtr 
domain,
  domain->name, to);
 goto cleanup;
 }
-if (safewrite(fd, TEST_SAVE_MAGIC, sizeof(TEST_SAVE_MAGIC)) < 0) {
+ret = asprintf(&buf, "%s_%u", TEST_SAVE_MAGIC, dumpformat);
+if (ret == -1) {
+goto cleanup;
+}
+size = ret;
+ret = -1;
+if (safewrite(fd, buf, size) < 0) {
 virReportSystemError(errno,
  _("domain '%1$s' coredump: failed to write header 
to %2$s"),
  domain->name, to);
@@ -2712,6 +2720,7 @@ static int testDomainCoreDumpWithFormat(virDomainPtr 
domain,
 
 ret = 0;
  cleanup:
+free(buf);
 VIR_FORCE_CLOSE(fd);
 virDomainObjEndAPI(&privdom);
 virObjectEventStateQueue(privconn->eventState, event);
-- 
2.30.2
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org