12:30 p.m.
While working on a patch to add dom0 support to the libxl driver,
I noticed a few existing bugs which are fixed by this small
series. IMO 1 and 2 are safe even during the freeze as they fix
quite annoying bugs. 3 and 4 are probably safe bug fixes too,
unless I'm confused about the use of
domainObj->{def,newDef,persistentDef}
Jim Fehlig (4):
libxl: don't overwrite domain state from statedir config
libxl: don't remove persistent domain on start failure
libxl: honor domainGetXMLDesc() --inactive flag
libxl: Set def->vcpus after successfully modifying live vcpu count
src/libxl/libxl_driver.c | 33 ++++++++++++++++++++++++++-------
1 file changed, 26 insertions(+), 7 deletions(-)
--
2.3.7
12:30 p.m.
New subject: [libvirt] [PATCH 1/4] libxl: don't overwrite domain state from statedir config
When restarting libvirtd and reconnecting to running domains,
libxlReconnectDomain() would unconditionally set the domain state
to VIR_DOMAIN_RUNNING, overwriting the state maintained in
$statedir/<domname>.xml. A domain in a paused state would have
the state changed to running, even though it was actually in a
paused state.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index a7be745..8a8d27d 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -372,8 +372,6 @@ libxlReconnectDomain(virDomainObjPtr vm,
vm->def, VIR_HOSTDEV_SP_PCI) < 0)
goto out;
- virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_UNKNOWN);
-
if (virAtomicIntInc(&driver->nactive) == 1 &&
driver->inhibitCallback)
driver->inhibitCallback(true, driver->inhibitOpaque);
--
2.3.7
12:30 p.m.
New subject: [libvirt] [PATCH 2/4] libxl: don't remove persistent domain on start failure
libxlDomainCreateXML() would remove a persistent domain if
libxlDomainStart() failed. Check if domain is persistent
before removing.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 8a8d27d..fb9523a 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -918,7 +918,10 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml,
if (libxlDomainStart(driver, vm, (flags & VIR_DOMAIN_START_PAUSED) != 0,
-1) < 0) {
- virDomainObjListRemove(driver->domains, vm);
+ if (!vm->persistent) {
+ virDomainObjListRemove(driver->domains, vm);
+ vm = NULL;
+ }
goto endjob;
}
--
2.3.7
12:30 p.m.
New subject: [libvirt] [PATCH 3/4] libxl: honor domainGetXMLDesc() --inactive flag
The libxl driver always uses virDomainObj->def when formatting
the domain XML description. Use virDomainObj->newDef when
--inactive flag is set.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index fb9523a..1a5b1a7 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2429,6 +2429,7 @@ static char *
libxlDomainGetXMLDesc(virDomainPtr dom, unsigned int flags)
{
virDomainObjPtr vm;
+ virDomainDefPtr def;
char *ret = NULL;
/* Flags checked by virDomainDefFormat */
@@ -2439,8 +2440,13 @@ libxlDomainGetXMLDesc(virDomainPtr dom, unsigned int flags)
if (virDomainGetXMLDescEnsureACL(dom->conn, vm->def, flags) < 0)
goto cleanup;
- ret = virDomainDefFormat(vm->def,
- virDomainDefFormatConvertXMLFlags(flags));
+ if ((flags & VIR_DOMAIN_XML_INACTIVE) && vm->newDef)
+ def = vm->newDef;
+ else
+ def = vm->def;
+
+ ret = virDomainDefFormat(def,
+ virDomainDefFormatConvertXMLFlags(flags));
cleanup:
if (vm)
--
2.3.7
12:30 p.m.
New subject: [libvirt] [PATCH 4/4] libxl: Set def->vcpus after successfully modifying live vcpu count
def->vcpus was never updated after successfully changing the live
vcpu count of a domain. Subsequent queries for vcpu info would
return incorrect results. E.g.:
virsh vcpucount test
maximum config 4
maximum live 4
current config 4
current live 4
virsh setvcpus test 2
virsh vcpucount test
maximum config 4
maximum live 4
current config 4
current live 4
After patch, live current config is reported correctly:
virsh vcpucount test
maximum config 4
maximum live 4
current config 4
current live 2
While fixing this, noticed that the live config was not saved
to cfg->stateDir via virDomainSaveStatus. Save the live config
and change error handling of virDomainSave{Config,Status} to
log a message via VIR_WARN, instead of failing the entire
DomainSetVcpusFlags operation.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_driver.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 1a5b1a7..96c9d96 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2101,6 +2101,7 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
" with libxenlight"), vm->def->id);
goto endjob;
}
+ vm->def->vcpus = nvcpus;
break;
case VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG:
@@ -2110,14 +2111,25 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
" with libxenlight"), vm->def->id);
goto endjob;
}
+ vm->def->vcpus = nvcpus;
def->vcpus = nvcpus;
break;
}
ret = 0;
- if (flags & VIR_DOMAIN_VCPU_CONFIG)
- ret = virDomainSaveConfig(cfg->configDir, def);
+ if (flags & VIR_DOMAIN_VCPU_LIVE) {
+ if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) {
+ VIR_WARN("Unable to save status on vm %s after changing vcpus",
+ vm->def->name);
+ }
+ }
+ if (flags & VIR_DOMAIN_VCPU_CONFIG) {
+ if (virDomainSaveConfig(cfg->configDir, def) < 0) {
+ VIR_WARN("Unable to save configuration of vm %s after changing
vcpus",
+ vm->def->name);
+ }
+ }
endjob:
if (!libxlDomainObjEndJob(driver, vm))
--
2.3.7
2:02 a.m.
On Mon, Jun 29, 2015 at 11:30:11AM -0600, Jim Fehlig wrote:
While working on a patch to add dom0 support to the libxl driver,
I noticed a few existing bugs which are fixed by this small
series. IMO 1 and 2 are safe even during the freeze as they fix
quite annoying bugs. 3 and 4 are probably safe bug fixes too,
unless I'm confused about the use of
domainObj->{def,newDef,persistentDef}
Jim Fehlig (4):
libxl: don't overwrite domain state from statedir config
libxl: don't remove persistent domain on start failure
libxl: honor domainGetXMLDesc() --inactive flag
libxl: Set def->vcpus after successfully modifying live vcpu count
src/libxl/libxl_driver.c | 33 ++++++++++++++++++++++++++-------
1 file changed, 26 insertions(+), 7 deletions(-)
Those looks reasonable to me but 1/ I'm a bit rusty 2/ locking is
hard so even if I ACK this take it with a grain of salt :-)
Daniel
--
Daniel Veillard | Open Source and Standards, Red Hat
veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | virtualization library http://libvirt.org/
5:08 a.m.
On 29.06.2015 19:30, Jim Fehlig wrote:
While working on a patch to add dom0 support to the libxl driver,
I noticed a few existing bugs which are fixed by this small
series. IMO 1 and 2 are safe even during the freeze as they fix
quite annoying bugs. 3 and 4 are probably safe bug fixes too,
unless I'm confused about the use of
domainObj->{def,newDef,persistentDef}
Jim Fehlig (4):
libxl: don't overwrite domain state from statedir config
libxl: don't remove persistent domain on start failure
libxl: honor domainGetXMLDesc() --inactive flag
libxl: Set def->vcpus after successfully modifying live vcpu count
src/libxl/libxl_driver.c | 33 ++++++++++++++++++++++++++-------
1 file changed, 26 insertions(+), 7 deletions(-)
ACK to all four and safe for the freeze.
Michal
1:14 p.m.
On 06/30/2015 04:08 AM, Michal Privoznik wrote:
On 29.06.2015 19:30, Jim Fehlig wrote:
> While working on a patch to add dom0 support to the libxl driver,
> I noticed a few existing bugs which are fixed by this small
> series. IMO 1 and 2 are safe even during the freeze as they fix
> quite annoying bugs. 3 and 4 are probably safe bug fixes too,
> unless I'm confused about the use of
> domainObj->{def,newDef,persistentDef}
>
> Jim Fehlig (4):
> libxl: don't overwrite domain state from statedir config
> libxl: don't remove persistent domain on start failure
> libxl: honor domainGetXMLDesc() --inactive flag
> libxl: Set def->vcpus after successfully modifying live vcpu count
>
> src/libxl/libxl_driver.c | 33 ++++++++++++++++++++++++++-------
> 1 file changed, 26 insertions(+), 7 deletions(-)
>
ACK to all four and safe for the freeze.
Thanks! I've pushed the series now.
Regards,
Jim
3433
days inactive
3434
days old
7 comments
3 participants
participants (3)
-
Daniel Veillard -
Jim Fehlig -
Michal Privoznik