Re: [libvirt] [PATCHv2.5 05/10] qemu: memdev: Add infrastructure to load memory device information
On Fri, Mar 13, 2015 at 10:38:51 -0400, John Ferlan wrote: > On 03/04/2015 11:24 AM, Peter Krempa wrote: > > When using 'dimm' memory devices with qemu, some of the information > > like the slot number and base address need to be reloaded from qemu > > after process start so that it reflects the actual state. The state then > > allows to use memory devices across migrations. > > --- > > src/qemu/qemu_domain.c | 49 + > > src/qemu/qemu_domain.h | 4 ++ > > src/qemu/qemu_monitor.c | 42 +++ > > src/qemu/qemu_monitor.h | 14 + > > src/qemu/qemu_monitor_json.c | 122 > > +++ > > src/qemu/qemu_monitor_json.h | 5 ++ > > src/qemu/qemu_process.c | 4 ++ > > 7 files changed, 240 insertions(+) ... > > > > + > > /* Technically, qemuProcessStart can be called from inside > > * QEMU_ASYNC_JOB_MIGRATION_IN, but we are okay treating this like > > * a sync job since no other job can call into the domain until > > > > > There's nothing through the qemuProcessAttach processing for this data > (although there is balloon info processing) > > - Decision on error handling of -2 or not All other places should handle well if qemu did not report the data. > - Don't drop into the loop to look at returned data if we had -2 returned I've added this, it would probably cause a crash. > - And add some sort of qemuProcessAttach handling... Since the command line parser is not implemeneted for memory devices and I don't really find it worth bothering with making qemuProcessAttach work with every new feature I would rather not try doing this. > > Just so it doesn't impede progress, I'm fine with a future follow-up > patch for the qemuProcessAttach. Leaving only handling the second point > above for an ACK Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2.5 05/10] qemu: memdev: Add infrastructure to load memory device information
On 03/04/2015 11:24 AM, Peter Krempa wrote:
> When using 'dimm' memory devices with qemu, some of the information
> like the slot number and base address need to be reloaded from qemu
> after process start so that it reflects the actual state. The state then
> allows to use memory devices across migrations.
> ---
> src/qemu/qemu_domain.c | 49 +
> src/qemu/qemu_domain.h | 4 ++
> src/qemu/qemu_monitor.c | 42 +++
> src/qemu/qemu_monitor.h | 14 +
> src/qemu/qemu_monitor_json.c | 122
> +++
> src/qemu/qemu_monitor_json.h | 5 ++
> src/qemu/qemu_process.c | 4 ++
> 7 files changed, 240 insertions(+)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 4225f38..61b0fc8 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2784,6 +2784,55 @@ qemuDomainUpdateDeviceList(virQEMUDriverPtr driver,
> return 0;
> }
>
> +
> +int
> +qemuDomainUpdateMemoryDeviceInfo(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + int asyncJob)
> +{
> +qemuDomainObjPrivatePtr priv = vm->privateData;
> +virHashTablePtr meminfo = NULL;
> +int rc;
> +size_t i;
> +
> +if (vm->def->nmems == 0)
> +return 0;
> +
> +if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
> +return -1;
> +
> +rc = qemuMonitorGetMemoryDeviceInfo(priv->mon, &meminfo);
> +
> +if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +return -1;
> +
> +/* if qemu doesn't support the info request, just carry on */
> +if (rc == -2)
> +rc = 0;
[1] hmmm
Cannot remember if we agreed previously that not having a message was ok
(e.g. "requires json" or "requires query-memory-devices command"). I
guess I figure if we get this far, then some other check has failed us.
But, but returning 0 we say - yep it worked, which of course isn't true.
I'm conflicted since future patches become affected...
> +
> +if (rc < 0)
> +return -1;
> +
> +for (i = 0; i < vm->def->nmems; i++) {
[1] If "the real" rc == -2, then vm->def->nmems > 0 and we enter this
loop which is probably not a good idea.
> +virDomainMemoryDefPtr mem = vm->def->mems[i];
> +qemuMonitorMemoryDeviceInfoPtr dimm;
> +
> +if (!mem->info.alias)
> +continue;
> +
> +if (!(dimm = virHashLookup(meminfo, mem->info.alias)))
> +continue;
> +
> +mem->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM;
> +mem->info.addr.dimm.slot = dimm->slot;
> +mem->info.addr.dimm.base = dimm->address;
> +}
> +
> +virHashFree(meminfo);
> +return 0;
> +}
> +
> +
> bool
> qemuDomainDefCheckABIStability(virQEMUDriverPtr driver,
> virDomainDefPtr src,
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 9760095..b2be323 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -391,6 +391,10 @@ extern virDomainDefParserConfig
> virQEMUDriverDomainDefParserConfig;
> int qemuDomainUpdateDeviceList(virQEMUDriverPtr driver,
> virDomainObjPtr vm, int asyncJob);
>
> +int qemuDomainUpdateMemoryDeviceInfo(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + int asyncJob);
> +
> bool qemuDomainDefCheckABIStability(virQEMUDriverPtr driver,
> virDomainDefPtr src,
> virDomainDefPtr dst);
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 94495cd..34673e1 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -4359,3 +4359,45 @@ void
> qemuMonitorIOThreadsInfoFree(qemuMonitorIOThreadsInfoPtr iothread)
> VIR_FREE(iothread->name);
> VIR_FREE(iothread);
> }
> +
> +
> +/**
> + * qemuMonitorGetMemoryDeviceInfo:
> + * @mon: pointer to the monitor
> + * @info: Location to return the hash of qemuMonitorMemoryDeviceInfo
> + *
> + * Retrieve state and addresses of frontend memory devices present in
> + * the guest.
> + *
> + * Returns 0 on success and fills @info with a newly allocated struct; if the
> + * data can't be retrieved due to lack of support in qemu, returns -2. On
> + * other errors returns -1.
> + */
> +int
> +qemuMonitorGetMemoryDeviceInfo(qemuMonitorPtr mon,
> + virHashTablePtr *info)
> +{
> +VIR_DEBUG("mon=%p info=%p", mon, info);
> +int ret;
> +
> +*info = NULL;
> +
> +if (!mon) {
> +virReportError(VIR_ERR_INVALID_ARG, "%s",
> + _("monitor must not be NULL"));
> +return -1;
> +}
> +
> +if (!mon->json)
> +return -2;
> +
> +if (!(*info = virHashCreate(10, virHashValueFree)))
> +return -1;
> +
> +if ((ret = qemuMonitorJSONGetMemoryDeviceInfo(mon, *i
[libvirt] [PATCHv2.5 05/10] qemu: memdev: Add infrastructure to load memory device information
When using 'dimm' memory devices with qemu, some of the information
like the slot number and base address need to be reloaded from qemu
after process start so that it reflects the actual state. The state then
allows to use memory devices across migrations.
---
src/qemu/qemu_domain.c | 49 +
src/qemu/qemu_domain.h | 4 ++
src/qemu/qemu_monitor.c | 42 +++
src/qemu/qemu_monitor.h | 14 +
src/qemu/qemu_monitor_json.c | 122 +++
src/qemu/qemu_monitor_json.h | 5 ++
src/qemu/qemu_process.c | 4 ++
7 files changed, 240 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 4225f38..61b0fc8 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2784,6 +2784,55 @@ qemuDomainUpdateDeviceList(virQEMUDriverPtr driver,
return 0;
}
+
+int
+qemuDomainUpdateMemoryDeviceInfo(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ int asyncJob)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+virHashTablePtr meminfo = NULL;
+int rc;
+size_t i;
+
+if (vm->def->nmems == 0)
+return 0;
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+return -1;
+
+rc = qemuMonitorGetMemoryDeviceInfo(priv->mon, &meminfo);
+
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+return -1;
+
+/* if qemu doesn't support the info request, just carry on */
+if (rc == -2)
+rc = 0;
+
+if (rc < 0)
+return -1;
+
+for (i = 0; i < vm->def->nmems; i++) {
+virDomainMemoryDefPtr mem = vm->def->mems[i];
+qemuMonitorMemoryDeviceInfoPtr dimm;
+
+if (!mem->info.alias)
+continue;
+
+if (!(dimm = virHashLookup(meminfo, mem->info.alias)))
+continue;
+
+mem->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM;
+mem->info.addr.dimm.slot = dimm->slot;
+mem->info.addr.dimm.base = dimm->address;
+}
+
+virHashFree(meminfo);
+return 0;
+}
+
+
bool
qemuDomainDefCheckABIStability(virQEMUDriverPtr driver,
virDomainDefPtr src,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 9760095..b2be323 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -391,6 +391,10 @@ extern virDomainDefParserConfig
virQEMUDriverDomainDefParserConfig;
int qemuDomainUpdateDeviceList(virQEMUDriverPtr driver,
virDomainObjPtr vm, int asyncJob);
+int qemuDomainUpdateMemoryDeviceInfo(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ int asyncJob);
+
bool qemuDomainDefCheckABIStability(virQEMUDriverPtr driver,
virDomainDefPtr src,
virDomainDefPtr dst);
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 94495cd..34673e1 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4359,3 +4359,45 @@ void
qemuMonitorIOThreadsInfoFree(qemuMonitorIOThreadsInfoPtr iothread)
VIR_FREE(iothread->name);
VIR_FREE(iothread);
}
+
+
+/**
+ * qemuMonitorGetMemoryDeviceInfo:
+ * @mon: pointer to the monitor
+ * @info: Location to return the hash of qemuMonitorMemoryDeviceInfo
+ *
+ * Retrieve state and addresses of frontend memory devices present in
+ * the guest.
+ *
+ * Returns 0 on success and fills @info with a newly allocated struct; if the
+ * data can't be retrieved due to lack of support in qemu, returns -2. On
+ * other errors returns -1.
+ */
+int
+qemuMonitorGetMemoryDeviceInfo(qemuMonitorPtr mon,
+ virHashTablePtr *info)
+{
+VIR_DEBUG("mon=%p info=%p", mon, info);
+int ret;
+
+*info = NULL;
+
+if (!mon) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+return -1;
+}
+
+if (!mon->json)
+return -2;
+
+if (!(*info = virHashCreate(10, virHashValueFree)))
+return -1;
+
+if ((ret = qemuMonitorJSONGetMemoryDeviceInfo(mon, *info)) < 0) {
+virHashFree(*info);
+*info = NULL;
+}
+
+return ret;
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 133d42d..811ce49 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -892,6 +892,20 @@ int qemuMonitorGetIOThreads(qemuMonitorPtr mon,
void qemuMonitorIOThreadsInfoFree(qemuMonitorIOThreadsInfoPtr iothread);
+typedef struct _qemuMonitorMemoryDeviceInfo qemuMonitorMemoryDeviceInfo;
+typedef qemuMonitorMemoryDeviceInfo *qemuMonitorMemoryDeviceInfoPtr;
+
+struct _qemuMonitorMemoryDeviceInfo {
+unsigned long long address;
+unsigned int slot;
+bool hotplugged;
+bool hotpluggable;
+};
+
+int qemuMonitorGetMemoryDeviceInfo(qemuMonitorPtr mon,
+ virHash
