Re: [libvirt] [PATCH V2 3/4] qemu: Add domain support for VCPU halted state
On 29.09.2016 16:34, John Ferlan wrote: [...] >> --- a/src/qemu/qemu_domain.c >> +++ b/src/qemu/qemu_domain.c >> @@ -5956,6 +5956,75 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver, >> return ret; >> } >> >> +/** >> + * qemuDomainGetVcpuHalted: >> + * @vm: domain object >> + * @vcpu: cpu id >> + * >> + * Returns the vCPU halted state. >> + */ >> +bool >> +qemuDomainGetVcpuHalted(virDomainObjPtr vm, >> +unsigned int vcpuid) >> +{ >> +virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, vcpuid); >> +return QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->halted; >> +} >> + >> +/** >> + * qemuDomainRefreshVcpuHalted: >> + * @driver: qemu driver data >> + * @vm: domain object >> + * @asyncJob: current asynchronous job type >> + * >> + * Updates vCPU halted state in the private data of @vm. >> + * >> + * Returns number of detected vCPUs on success, -1 on error and reports >> + * an appropriate error, -2 if the domain doesn't exist any more. > > Neither of the callers checks -1 or -2, just < 0, so is this really > necessary? > yes, this is nonsense, IIRC this was needed in the first version of the series to differentiate the two cases >> + */ >> +int >> +qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver, >> +virDomainObjPtr vm, >> +int asyncJob) >> +{ >> +virDomainVcpuDefPtr vcpu; >> +qemuMonitorCPUInfoPtr info = NULL; >> +size_t maxvcpus = virDomainDefGetVcpusMax(vm->def); >> +size_t i; >> +bool hotplug; >> +int rc; >> +int ret = -1; >> + >> +/* Not supported currently for TCG, see qemuDomainRefreshVcpuInfo */ >> +if (vm->def->virtType == VIR_DOMAIN_VIRT_QEMU) >> +return 0; > > Since the "default" is halted = true could we run into a situation where > "halted" (or "running (inactive)") is always set for TCG... > good point, "halted" as the new kid in town should actually be false by default, as you have observed the issue is introduced in patch 2/4 (monitors). [...] -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V2 3/4] qemu: Add domain support for VCPU halted state
On 09/20/2016 04:11 AM, Viktor Mihajlovski wrote: > Adding a field to the domain's private vcpu object to hold the halted > state information. > Adding two functions in support of the halted state: > - qemuDomainGetVcpuHalted: retrieve the halted state from a > private vcpu object > - qemuDomainRefreshVcpuHalted: obtain the per-vcpu halted states > via qemu monitor and store the results in the private vcpu objects > > Signed-off-by: Viktor Mihajlovski> Reviewed-by: Bjoern Walk > Reviewed-by: Hao QingFeng > Signed-off-by: Boris Fiuczynski > --- > src/qemu/qemu_domain.c | 69 > ++ > src/qemu/qemu_domain.h | 5 > 2 files changed, 74 insertions(+) > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 3f16dbe..3fb9b4f 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -5956,6 +5956,75 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver, > return ret; > } > > +/** > + * qemuDomainGetVcpuHalted: > + * @vm: domain object > + * @vcpu: cpu id > + * > + * Returns the vCPU halted state. > + */ > +bool > +qemuDomainGetVcpuHalted(virDomainObjPtr vm, > +unsigned int vcpuid) > +{ > +virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, vcpuid); > +return QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->halted; > +} > + > +/** > + * qemuDomainRefreshVcpuHalted: > + * @driver: qemu driver data > + * @vm: domain object > + * @asyncJob: current asynchronous job type > + * > + * Updates vCPU halted state in the private data of @vm. > + * > + * Returns number of detected vCPUs on success, -1 on error and reports > + * an appropriate error, -2 if the domain doesn't exist any more. Neither of the callers checks -1 or -2, just < 0, so is this really necessary? > + */ > +int > +qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver, > +virDomainObjPtr vm, > +int asyncJob) > +{ > +virDomainVcpuDefPtr vcpu; > +qemuMonitorCPUInfoPtr info = NULL; > +size_t maxvcpus = virDomainDefGetVcpusMax(vm->def); > +size_t i; > +bool hotplug; > +int rc; > +int ret = -1; > + > +/* Not supported currently for TCG, see qemuDomainRefreshVcpuInfo */ > +if (vm->def->virtType == VIR_DOMAIN_VIRT_QEMU) > +return 0; Since the "default" is halted = true could we run into a situation where "halted" (or "running (inactive)") is always set for TCG... > + > +hotplug = qemuDomainSupportsNewVcpuHotplug(vm); > + > +if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) > +return -1; > + > +rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), , maxvcpus, > hotplug); > + > +if (qemuDomainObjExitMonitor(driver, vm) < 0) { > +ret = -2; I see no need to ret = -2; > +goto cleanup; > +} > + > +if (rc < 0) > +goto cleanup; > + > +for (i = 0; i < maxvcpus; i++) { > +vcpu = virDomainDefGetVcpu(vm->def, i); > +QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->halted = info[i].halted; > +} > + > +ret = 0; > + > + cleanup: > +qemuMonitorCPUInfoFree(info, maxvcpus); > +return ret; > +} > > bool > qemuDomainSupportsNicdev(virDomainDefPtr def, > diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h > index a1404d0..03e58c5 100644 > --- a/src/qemu/qemu_domain.h > +++ b/src/qemu/qemu_domain.h > @@ -317,6 +317,7 @@ struct _qemuDomainVcpuPrivate { > pid_t tid; /* vcpu thread id */ > int enable_id; /* order in which the vcpus were enabled in qemu */ > char *alias; > +bool halted; /* vcpu halted state */ Another less than necessary comment ;-) John > > /* information for hotpluggable cpus */ > char *type; > @@ -662,6 +663,10 @@ int qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver, >virDomainObjPtr vm, >int asyncJob, >bool state); > +bool qemuDomainGetVcpuHalted(virDomainObjPtr vm, unsigned int vcpu); > +int qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver, > +virDomainObjPtr vm, > +int asyncJob); > > bool qemuDomainSupportsNicdev(virDomainDefPtr def, >virDomainNetDefPtr net); > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH V2 3/4] qemu: Add domain support for VCPU halted state
Adding a field to the domain's private vcpu object to hold the halted state information. Adding two functions in support of the halted state: - qemuDomainGetVcpuHalted: retrieve the halted state from a private vcpu object - qemuDomainRefreshVcpuHalted: obtain the per-vcpu halted states via qemu monitor and store the results in the private vcpu objects Signed-off-by: Viktor MihajlovskiReviewed-by: Bjoern Walk Reviewed-by: Hao QingFeng Signed-off-by: Boris Fiuczynski --- src/qemu/qemu_domain.c | 69 ++ src/qemu/qemu_domain.h | 5 2 files changed, 74 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3f16dbe..3fb9b4f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5956,6 +5956,75 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver, return ret; } +/** + * qemuDomainGetVcpuHalted: + * @vm: domain object + * @vcpu: cpu id + * + * Returns the vCPU halted state. + */ +bool +qemuDomainGetVcpuHalted(virDomainObjPtr vm, +unsigned int vcpuid) +{ +virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, vcpuid); +return QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->halted; +} + +/** + * qemuDomainRefreshVcpuHalted: + * @driver: qemu driver data + * @vm: domain object + * @asyncJob: current asynchronous job type + * + * Updates vCPU halted state in the private data of @vm. + * + * Returns number of detected vCPUs on success, -1 on error and reports + * an appropriate error, -2 if the domain doesn't exist any more. + */ +int +qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver, +virDomainObjPtr vm, +int asyncJob) +{ +virDomainVcpuDefPtr vcpu; +qemuMonitorCPUInfoPtr info = NULL; +size_t maxvcpus = virDomainDefGetVcpusMax(vm->def); +size_t i; +bool hotplug; +int rc; +int ret = -1; + +/* Not supported currently for TCG, see qemuDomainRefreshVcpuInfo */ +if (vm->def->virtType == VIR_DOMAIN_VIRT_QEMU) +return 0; + +hotplug = qemuDomainSupportsNewVcpuHotplug(vm); + +if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) +return -1; + +rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), , maxvcpus, hotplug); + +if (qemuDomainObjExitMonitor(driver, vm) < 0) { +ret = -2; +goto cleanup; +} + +if (rc < 0) +goto cleanup; + +for (i = 0; i < maxvcpus; i++) { +vcpu = virDomainDefGetVcpu(vm->def, i); +QEMU_DOMAIN_VCPU_PRIVATE(vcpu)->halted = info[i].halted; +} + +ret = 0; + + cleanup: +qemuMonitorCPUInfoFree(info, maxvcpus); +return ret; +} bool qemuDomainSupportsNicdev(virDomainDefPtr def, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index a1404d0..03e58c5 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -317,6 +317,7 @@ struct _qemuDomainVcpuPrivate { pid_t tid; /* vcpu thread id */ int enable_id; /* order in which the vcpus were enabled in qemu */ char *alias; +bool halted; /* vcpu halted state */ /* information for hotpluggable cpus */ char *type; @@ -662,6 +663,10 @@ int qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob, bool state); +bool qemuDomainGetVcpuHalted(virDomainObjPtr vm, unsigned int vcpu); +int qemuDomainRefreshVcpuHalted(virQEMUDriverPtr driver, +virDomainObjPtr vm, +int asyncJob); bool qemuDomainSupportsNicdev(virDomainDefPtr def, virDomainNetDefPtr net); -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list