[PATCH] support for hotplug/hotunplug in test hypervisor
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
> -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
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
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
> -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
> -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
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
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
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
> -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
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
> -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
> -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
> -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
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
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
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
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
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
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
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