* [Qemu-devel] [PATCH v4] spapr: add uuid/host details to device tree
@ 2014-07-08 5:00 Nikunj A Dadhania
2014-07-08 10:49 ` Alexander Graf
0 siblings, 1 reply; 6+ messages in thread
From: Nikunj A Dadhania @ 2014-07-08 5:00 UTC (permalink / raw)
To: qemu-devel, agraf; +Cc: aik, qemu-ppc, nikunj
Useful for identifying the guest/host uniquely within the
guest. Adding following properties to the guest root node.
vm,uuid - uuid of the guest
host-model - Host model number
host-serial - Host machine serial number
hypervisor type - Tells its "kvm"
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
v4: make uuid as human readable
v3: rebase to ppcnext
v2: indentation fixes
---
hw/ppc/spapr.c | 25 +++++++++++++++++++++++++
target-ppc/kvm.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
target-ppc/kvm_ppc.h | 12 ++++++++++++
3 files changed, 80 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 077ad2d..485ea66 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -318,6 +318,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
QemuOpts *opts = qemu_opts_find(qemu_find_opts("smp-opts"), NULL);
unsigned sockets = opts ? qemu_opt_get_number(opts, "sockets", 0) : 0;
uint32_t cpus_per_socket = sockets ? (smp_cpus / sockets) : 1;
+ char char_buf[512];
add_str(hypertas, "hcall-pft");
add_str(hypertas, "hcall-term");
@@ -347,6 +348,30 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
_FDT((fdt_property_string(fdt, "model", "IBM pSeries (emulated by qemu)")));
_FDT((fdt_property_string(fdt, "compatible", "qemu,pseries")));
+ if (kvm_enabled()) {
+ _FDT((fdt_property_string(fdt, "hypervisor", "kvm")));
+ }
+
+ /*
+ * Add info to guest to indentify which host is it being run on
+ * and what is the uuid of the guest
+ */
+ memset(char_buf, 0, sizeof(char_buf));
+ if (!kvmppc_get_host_model(char_buf, sizeof(char_buf))) {
+ _FDT((fdt_property_string(fdt, "host-model", char_buf)));
+ memset(char_buf, 0, sizeof(char_buf));
+ }
+ if (!kvmppc_get_host_serial(char_buf, sizeof(char_buf))) {
+ _FDT((fdt_property_string(fdt, "host-serial", char_buf)));
+ }
+
+ snprintf(char_buf, 37, UUID_FMT, qemu_uuid[0], qemu_uuid[1],
+ qemu_uuid[2], qemu_uuid[3], qemu_uuid[4], qemu_uuid[5],
+ qemu_uuid[6], qemu_uuid[7], qemu_uuid[8], qemu_uuid[9],
+ qemu_uuid[10], qemu_uuid[11], qemu_uuid[12], qemu_uuid[13],
+ qemu_uuid[14], qemu_uuid[15]);
+ _FDT((fdt_property_string(fdt, "vm,uuid", char_buf)));
+
_FDT((fdt_property_cell(fdt, "#address-cells", 0x2)));
_FDT((fdt_property_cell(fdt, "#size-cells", 0x2)));
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 2d87108..25091f8 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1369,7 +1369,7 @@ static int read_cpuinfo(const char *field, char *value, int len)
}
do {
- if(!fgets(line, sizeof(line), f)) {
+ if (!fgets(line, sizeof(line), f)) {
break;
}
if (!strncmp(line, field, field_len)) {
@@ -1404,6 +1404,48 @@ uint32_t kvmppc_get_tbfreq(void)
return retval;
}
+int32_t kvmppc_get_host_serial(char *value, int len)
+{
+ FILE *f;
+ int ret = -1;
+ char line[512];
+
+ memset(line, 0, sizeof(line));
+ f = fopen("/proc/device-tree/system-id", "r");
+ if (!f) {
+ return ret;
+ }
+
+ if (fgets(line, sizeof(line), f)) {
+ snprintf(value, len, "IBM,%s", line);
+ ret = 0;
+ }
+ fclose(f);
+
+ return ret;
+}
+
+int32_t kvmppc_get_host_model(char *value, int len)
+{
+ FILE *f;
+ int ret = -1;
+ char line[512];
+
+ memset(line, 0, sizeof(line));
+ f = fopen("/proc/device-tree/model", "r");
+ if (!f) {
+ return ret;
+ }
+
+ if (fgets(line, sizeof(line), f)) {
+ snprintf(value, len, "IBM,%s", line);
+ ret = 0;
+ }
+ fclose(f);
+
+ return ret;
+}
+
/* Try to find a device tree node for a CPU with clock-frequency property */
static int kvmppc_find_cpu_dt(char *buf, int buf_len)
{
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index 1118122..6fa3314 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -19,6 +19,8 @@ uint32_t kvmppc_get_tbfreq(void);
uint64_t kvmppc_get_clockfreq(void);
uint32_t kvmppc_get_vmx(void);
uint32_t kvmppc_get_dfp(void);
+int32_t kvmppc_get_host_model(char *buf, int buf_len);
+int32_t kvmppc_get_host_serial(char *buf, int buf_len);
int kvmppc_get_hasidle(CPUPPCState *env);
int kvmppc_get_hypercall(CPUPPCState *env, uint8_t *buf, int buf_len);
int kvmppc_set_interrupt(PowerPCCPU *cpu, int irq, int level);
@@ -60,6 +62,16 @@ static inline uint32_t kvmppc_get_tbfreq(void)
return 0;
}
+static inline int32_t kvmppc_get_host_model(char *buf, int buf_len)
+{
+ return 0;
+}
+
+static inline int32_t kvmppc_get_host_serial(char *buf, int buf_len)
+{
+ return 0;
+}
+
static inline uint64_t kvmppc_get_clockfreq(void)
{
return 0;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v4] spapr: add uuid/host details to device tree
2014-07-08 5:00 [Qemu-devel] [PATCH v4] spapr: add uuid/host details to device tree Nikunj A Dadhania
@ 2014-07-08 10:49 ` Alexander Graf
2014-07-08 10:55 ` [Qemu-devel] [Qemu-ppc] " Benjamin Herrenschmidt
2014-07-08 11:04 ` [Qemu-devel] " Nikunj A Dadhania
0 siblings, 2 replies; 6+ messages in thread
From: Alexander Graf @ 2014-07-08 10:49 UTC (permalink / raw)
To: Nikunj A Dadhania, qemu-devel; +Cc: aik, qemu-ppc
On 08.07.14 07:00, Nikunj A Dadhania wrote:
> Useful for identifying the guest/host uniquely within the
> guest. Adding following properties to the guest root node.
>
> vm,uuid - uuid of the guest
> host-model - Host model number
> host-serial - Host machine serial number
> hypervisor type - Tells its "kvm"
>
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>
> ---
> v4: make uuid as human readable
> v3: rebase to ppcnext
> v2: indentation fixes
> ---
> hw/ppc/spapr.c | 25 +++++++++++++++++++++++++
> target-ppc/kvm.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
> target-ppc/kvm_ppc.h | 12 ++++++++++++
> 3 files changed, 80 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 077ad2d..485ea66 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -318,6 +318,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
> QemuOpts *opts = qemu_opts_find(qemu_find_opts("smp-opts"), NULL);
> unsigned sockets = opts ? qemu_opt_get_number(opts, "sockets", 0) : 0;
> uint32_t cpus_per_socket = sockets ? (smp_cpus / sockets) : 1;
> + char char_buf[512];
Can't you just return callee allocated, caller free'd memory?
>
> add_str(hypertas, "hcall-pft");
> add_str(hypertas, "hcall-term");
> @@ -347,6 +348,30 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
> _FDT((fdt_property_string(fdt, "model", "IBM pSeries (emulated by qemu)")));
> _FDT((fdt_property_string(fdt, "compatible", "qemu,pseries")));
>
> + if (kvm_enabled()) {
> + _FDT((fdt_property_string(fdt, "hypervisor", "kvm")));
> + }
> +
> + /*
> + * Add info to guest to indentify which host is it being run on
> + * and what is the uuid of the guest
> + */
> + memset(char_buf, 0, sizeof(char_buf));
> + if (!kvmppc_get_host_model(char_buf, sizeof(char_buf))) {
> + _FDT((fdt_property_string(fdt, "host-model", char_buf)));
> + memset(char_buf, 0, sizeof(char_buf));
> + }
> + if (!kvmppc_get_host_serial(char_buf, sizeof(char_buf))) {
> + _FDT((fdt_property_string(fdt, "host-serial", char_buf)));
> + }
Please be aware that all of the above is bogus when you start thinking
about live migration.
> +
> + snprintf(char_buf, 37, UUID_FMT, qemu_uuid[0], qemu_uuid[1],
g_strdup_printf()
> + qemu_uuid[2], qemu_uuid[3], qemu_uuid[4], qemu_uuid[5],
> + qemu_uuid[6], qemu_uuid[7], qemu_uuid[8], qemu_uuid[9],
> + qemu_uuid[10], qemu_uuid[11], qemu_uuid[12], qemu_uuid[13],
> + qemu_uuid[14], qemu_uuid[15]);
> + _FDT((fdt_property_string(fdt, "vm,uuid", char_buf)));
> +
> _FDT((fdt_property_cell(fdt, "#address-cells", 0x2)));
> _FDT((fdt_property_cell(fdt, "#size-cells", 0x2)));
>
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 2d87108..25091f8 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1369,7 +1369,7 @@ static int read_cpuinfo(const char *field, char *value, int len)
> }
>
> do {
> - if(!fgets(line, sizeof(line), f)) {
> + if (!fgets(line, sizeof(line), f)) {
> break;
> }
> if (!strncmp(line, field, field_len)) {
> @@ -1404,6 +1404,48 @@ uint32_t kvmppc_get_tbfreq(void)
> return retval;
> }
>
> +int32_t kvmppc_get_host_serial(char *value, int len)
> +{
> + FILE *f;
> + int ret = -1;
> + char line[512];
> +
> + memset(line, 0, sizeof(line));
> + f = fopen("/proc/device-tree/system-id", "r");
> + if (!f) {
> + return ret;
> + }
> +
> + if (fgets(line, sizeof(line), f)) {
> + snprintf(value, len, "IBM,%s", line);
Why IBM,<system-id>?
> + ret = 0;
> + }
> + fclose(f);
> +
> + return ret;
I think it makes sense to extract the "read a full file into a buffer"
logic into a separate function. For bonus points, find a glib function
that already does it and use that ;).
> +}
> +
> +int32_t kvmppc_get_host_model(char *value, int len)
> +{
> + FILE *f;
> + int ret = -1;
> + char line[512];
> +
> + memset(line, 0, sizeof(line));
> + f = fopen("/proc/device-tree/model", "r");
> + if (!f) {
> + return ret;
> + }
> +
> + if (fgets(line, sizeof(line), f)) {
> + snprintf(value, len, "IBM,%s", line);
Same here - wouldn't this be IBM,IBM,foo?
Alex
> + ret = 0;
> + }
> + fclose(f);
> +
> + return ret;
> +}
> +
> /* Try to find a device tree node for a CPU with clock-frequency property */
> static int kvmppc_find_cpu_dt(char *buf, int buf_len)
> {
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index 1118122..6fa3314 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -19,6 +19,8 @@ uint32_t kvmppc_get_tbfreq(void);
> uint64_t kvmppc_get_clockfreq(void);
> uint32_t kvmppc_get_vmx(void);
> uint32_t kvmppc_get_dfp(void);
> +int32_t kvmppc_get_host_model(char *buf, int buf_len);
> +int32_t kvmppc_get_host_serial(char *buf, int buf_len);
> int kvmppc_get_hasidle(CPUPPCState *env);
> int kvmppc_get_hypercall(CPUPPCState *env, uint8_t *buf, int buf_len);
> int kvmppc_set_interrupt(PowerPCCPU *cpu, int irq, int level);
> @@ -60,6 +62,16 @@ static inline uint32_t kvmppc_get_tbfreq(void)
> return 0;
> }
>
> +static inline int32_t kvmppc_get_host_model(char *buf, int buf_len)
> +{
> + return 0;
> +}
> +
> +static inline int32_t kvmppc_get_host_serial(char *buf, int buf_len)
> +{
> + return 0;
> +}
> +
> static inline uint64_t kvmppc_get_clockfreq(void)
> {
> return 0;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH v4] spapr: add uuid/host details to device tree
2014-07-08 10:49 ` Alexander Graf
@ 2014-07-08 10:55 ` Benjamin Herrenschmidt
2014-07-08 11:04 ` [Qemu-devel] " Nikunj A Dadhania
1 sibling, 0 replies; 6+ messages in thread
From: Benjamin Herrenschmidt @ 2014-07-08 10:55 UTC (permalink / raw)
To: Alexander Graf; +Cc: qemu-ppc, qemu-devel, Nikunj A Dadhania
On Tue, 2014-07-08 at 12:49 +0200, Alexander Graf wrote:
> Please be aware that all of the above is bogus when you start
> thinking
> about live migration.
What's probably where we need to start thinking about implementing
migration according to PAPR :-)
IE. With pre and post-migration notifications to the guest including
device-tree updates.
Cheers,
Ben.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v4] spapr: add uuid/host details to device tree
2014-07-08 10:49 ` Alexander Graf
2014-07-08 10:55 ` [Qemu-devel] [Qemu-ppc] " Benjamin Herrenschmidt
@ 2014-07-08 11:04 ` Nikunj A Dadhania
2014-07-08 11:11 ` Alexander Graf
1 sibling, 1 reply; 6+ messages in thread
From: Nikunj A Dadhania @ 2014-07-08 11:04 UTC (permalink / raw)
To: Alexander Graf, qemu-devel; +Cc: aik, qemu-ppc
Alexander Graf <agraf@suse.de> writes:
> On 08.07.14 07:00, Nikunj A Dadhania wrote:
>> Useful for identifying the guest/host uniquely within the
>> guest. Adding following properties to the guest root node.
>>
>> vm,uuid - uuid of the guest
>> host-model - Host model number
>> host-serial - Host machine serial number
>> hypervisor type - Tells its "kvm"
>>
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>>
>> ---
>> v4: make uuid as human readable
>> v3: rebase to ppcnext
>> v2: indentation fixes
>> ---
>> hw/ppc/spapr.c | 25 +++++++++++++++++++++++++
>> target-ppc/kvm.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
>> target-ppc/kvm_ppc.h | 12 ++++++++++++
>> 3 files changed, 80 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 077ad2d..485ea66 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -318,6 +318,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>> QemuOpts *opts = qemu_opts_find(qemu_find_opts("smp-opts"), NULL);
>> unsigned sockets = opts ? qemu_opt_get_number(opts, "sockets", 0) : 0;
>> uint32_t cpus_per_socket = sockets ? (smp_cpus / sockets) : 1;
>> + char char_buf[512];
>
> Can't you just return callee allocated, caller free'd memory?
Tried doing it more in line of read_cpuinfo in target-ppc/kvm.c
I could do it either ways.
>
>>
>> add_str(hypertas, "hcall-pft");
>> add_str(hypertas, "hcall-term");
>> @@ -347,6 +348,30 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>> _FDT((fdt_property_string(fdt, "model", "IBM pSeries (emulated by qemu)")));
>> _FDT((fdt_property_string(fdt, "compatible", "qemu,pseries")));
>>
>> + if (kvm_enabled()) {
>> + _FDT((fdt_property_string(fdt, "hypervisor", "kvm")));
>> + }
>> +
>> + /*
>> + * Add info to guest to indentify which host is it being run on
>> + * and what is the uuid of the guest
>> + */
>> + memset(char_buf, 0, sizeof(char_buf));
>> + if (!kvmppc_get_host_model(char_buf, sizeof(char_buf))) {
>> + _FDT((fdt_property_string(fdt, "host-model", char_buf)));
>> + memset(char_buf, 0, sizeof(char_buf));
>> + }
>> + if (!kvmppc_get_host_serial(char_buf, sizeof(char_buf))) {
>> + _FDT((fdt_property_string(fdt, "host-serial", char_buf)));
>> + }
>
> Please be aware that all of the above is bogus when you start thinking
> about live migration.
Yes, there are tools that look at these. Is there a way to update these
on migration?
>
>> +
>> + snprintf(char_buf, 37, UUID_FMT, qemu_uuid[0], qemu_uuid[1],
>
> g_strdup_printf()
Ok.
>
>> + qemu_uuid[2], qemu_uuid[3], qemu_uuid[4], qemu_uuid[5],
>> + qemu_uuid[6], qemu_uuid[7], qemu_uuid[8], qemu_uuid[9],
>> + qemu_uuid[10], qemu_uuid[11], qemu_uuid[12], qemu_uuid[13],
>> + qemu_uuid[14], qemu_uuid[15]);
>> + _FDT((fdt_property_string(fdt, "vm,uuid", char_buf)));
>> +
>> _FDT((fdt_property_cell(fdt, "#address-cells", 0x2)));
>> _FDT((fdt_property_cell(fdt, "#size-cells", 0x2)));
>>
>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>> index 2d87108..25091f8 100644
>> --- a/target-ppc/kvm.c
>> +++ b/target-ppc/kvm.c
>> @@ -1369,7 +1369,7 @@ static int read_cpuinfo(const char *field, char *value, int len)
>> }
>>
>> do {
>> - if(!fgets(line, sizeof(line), f)) {
>> + if (!fgets(line, sizeof(line), f)) {
>> break;
>> }
>> if (!strncmp(line, field, field_len)) {
>> @@ -1404,6 +1404,48 @@ uint32_t kvmppc_get_tbfreq(void)
>> return retval;
>> }
>>
>> +int32_t kvmppc_get_host_serial(char *value, int len)
>> +{
>> + FILE *f;
>> + int ret = -1;
>> + char line[512];
>> +
>> + memset(line, 0, sizeof(line));
>> + f = fopen("/proc/device-tree/system-id", "r");
>> + if (!f) {
>> + return ret;
>> + }
>> +
>> + if (fgets(line, sizeof(line), f)) {
>> + snprintf(value, len, "IBM,%s", line);
>
> Why IBM,<system-id>?
There were userspace tools that looking at lparcfg, and were encoded
similarly.
>
>> + ret = 0;
>> + }
>> + fclose(f);
>> +
>> + return ret;
>
> I think it makes sense to extract the "read a full file into a buffer"
> logic into a separate function. For bonus points, find a glib function
> that already does it and use that ;).
Let me search.
>
>> +}
>> +
>> +int32_t kvmppc_get_host_model(char *value, int len)
>> +{
>> + FILE *f;
>> + int ret = -1;
>> + char line[512];
>> +
>> + memset(line, 0, sizeof(line));
>> + f = fopen("/proc/device-tree/model", "r");
>> + if (!f) {
>> + return ret;
>> + }
>> +
>> + if (fgets(line, sizeof(line), f)) {
>> + snprintf(value, len, "IBM,%s", line);
>
> Same here - wouldn't this be IBM,IBM,foo?
No, it will be IBM,<model>
Regards
Nikunj
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v4] spapr: add uuid/host details to device tree
2014-07-08 11:04 ` [Qemu-devel] " Nikunj A Dadhania
@ 2014-07-08 11:11 ` Alexander Graf
2014-07-08 11:26 ` Nikunj A Dadhania
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Graf @ 2014-07-08 11:11 UTC (permalink / raw)
To: Nikunj A Dadhania, qemu-devel; +Cc: aik, qemu-ppc
On 08.07.14 13:04, Nikunj A Dadhania wrote:
> Alexander Graf <agraf@suse.de> writes:
>
>> On 08.07.14 07:00, Nikunj A Dadhania wrote:
>>> Useful for identifying the guest/host uniquely within the
>>> guest. Adding following properties to the guest root node.
>>>
>>> vm,uuid - uuid of the guest
>>> host-model - Host model number
>>> host-serial - Host machine serial number
>>> hypervisor type - Tells its "kvm"
>>>
>>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>>>
>>> ---
>>> v4: make uuid as human readable
>>> v3: rebase to ppcnext
>>> v2: indentation fixes
>>> ---
>>> hw/ppc/spapr.c | 25 +++++++++++++++++++++++++
>>> target-ppc/kvm.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
>>> target-ppc/kvm_ppc.h | 12 ++++++++++++
>>> 3 files changed, 80 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index 077ad2d..485ea66 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -318,6 +318,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>>> QemuOpts *opts = qemu_opts_find(qemu_find_opts("smp-opts"), NULL);
>>> unsigned sockets = opts ? qemu_opt_get_number(opts, "sockets", 0) : 0;
>>> uint32_t cpus_per_socket = sockets ? (smp_cpus / sockets) : 1;
>>> + char char_buf[512];
>> Can't you just return callee allocated, caller free'd memory?
> Tried doing it more in line of read_cpuinfo in target-ppc/kvm.c
> I could do it either ways.
Yeah, feel free to convert that one too if you like ;).
>
>>>
>>> add_str(hypertas, "hcall-pft");
>>> add_str(hypertas, "hcall-term");
>>> @@ -347,6 +348,30 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>>> _FDT((fdt_property_string(fdt, "model", "IBM pSeries (emulated by qemu)")));
>>> _FDT((fdt_property_string(fdt, "compatible", "qemu,pseries")));
>>>
>>> + if (kvm_enabled()) {
>>> + _FDT((fdt_property_string(fdt, "hypervisor", "kvm")));
>>> + }
>>> +
>>> + /*
>>> + * Add info to guest to indentify which host is it being run on
>>> + * and what is the uuid of the guest
>>> + */
>>> + memset(char_buf, 0, sizeof(char_buf));
>>> + if (!kvmppc_get_host_model(char_buf, sizeof(char_buf))) {
>>> + _FDT((fdt_property_string(fdt, "host-model", char_buf)));
>>> + memset(char_buf, 0, sizeof(char_buf));
>>> + }
>>> + if (!kvmppc_get_host_serial(char_buf, sizeof(char_buf))) {
>>> + _FDT((fdt_property_string(fdt, "host-serial", char_buf)));
>>> + }
>> Please be aware that all of the above is bogus when you start thinking
>> about live migration.
> Yes, there are tools that look at these. Is there a way to update these
> on migration?
As Ben already mentioned ;).
>
>>> +
>>> + snprintf(char_buf, 37, UUID_FMT, qemu_uuid[0], qemu_uuid[1],
>> g_strdup_printf()
> Ok.
>
>>> + qemu_uuid[2], qemu_uuid[3], qemu_uuid[4], qemu_uuid[5],
>>> + qemu_uuid[6], qemu_uuid[7], qemu_uuid[8], qemu_uuid[9],
>>> + qemu_uuid[10], qemu_uuid[11], qemu_uuid[12], qemu_uuid[13],
>>> + qemu_uuid[14], qemu_uuid[15]);
>>> + _FDT((fdt_property_string(fdt, "vm,uuid", char_buf)));
>>> +
>>> _FDT((fdt_property_cell(fdt, "#address-cells", 0x2)));
>>> _FDT((fdt_property_cell(fdt, "#size-cells", 0x2)));
>>>
>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>>> index 2d87108..25091f8 100644
>>> --- a/target-ppc/kvm.c
>>> +++ b/target-ppc/kvm.c
>>> @@ -1369,7 +1369,7 @@ static int read_cpuinfo(const char *field, char *value, int len)
>>> }
>>>
>>> do {
>>> - if(!fgets(line, sizeof(line), f)) {
>>> + if (!fgets(line, sizeof(line), f)) {
>>> break;
>>> }
>>> if (!strncmp(line, field, field_len)) {
>>> @@ -1404,6 +1404,48 @@ uint32_t kvmppc_get_tbfreq(void)
>>> return retval;
>>> }
>>>
>>> +int32_t kvmppc_get_host_serial(char *value, int len)
>>> +{
>>> + FILE *f;
>>> + int ret = -1;
>>> + char line[512];
>>> +
>>> + memset(line, 0, sizeof(line));
>>> + f = fopen("/proc/device-tree/system-id", "r");
>>> + if (!f) {
>>> + return ret;
>>> + }
>>> +
>>> + if (fgets(line, sizeof(line), f)) {
>>> + snprintf(value, len, "IBM,%s", line);
>> Why IBM,<system-id>?
> There were userspace tools that looking at lparcfg, and were encoded
> similarly.
I don't think we own the IBM namespace, so I find this slightly bogus.
Also why would a host machine have to be made by IBM?
>
>>> + ret = 0;
>>> + }
>>> + fclose(f);
>>> +
>>> + return ret;
>> I think it makes sense to extract the "read a full file into a buffer"
>> logic into a separate function. For bonus points, find a glib function
>> that already does it and use that ;).
> Let me search.
>
>>> +}
>>> +
>>> +int32_t kvmppc_get_host_model(char *value, int len)
>>> +{
>>> + FILE *f;
>>> + int ret = -1;
>>> + char line[512];
>>> +
>>> + memset(line, 0, sizeof(line));
>>> + f = fopen("/proc/device-tree/model", "r");
>>> + if (!f) {
>>> + return ret;
>>> + }
>>> +
>>> + if (fgets(line, sizeof(line), f)) {
>>> + snprintf(value, len, "IBM,%s", line);
>> Same here - wouldn't this be IBM,IBM,foo?
> No, it will be IBM,<model>
Hrm, I just tried to compare this with a pHyp system and can't seem to
find any /proc/device-tree/host* files. What am I missing?
Alex
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v4] spapr: add uuid/host details to device tree
2014-07-08 11:11 ` Alexander Graf
@ 2014-07-08 11:26 ` Nikunj A Dadhania
0 siblings, 0 replies; 6+ messages in thread
From: Nikunj A Dadhania @ 2014-07-08 11:26 UTC (permalink / raw)
To: Alexander Graf, qemu-devel; +Cc: aik, qemu-ppc
Alexander Graf <agraf@suse.de> writes:
> On 08.07.14 13:04, Nikunj A Dadhania wrote:
>> Alexander Graf <agraf@suse.de> writes:
>>
>>> On 08.07.14 07:00, Nikunj A Dadhania wrote:
>>>> Useful for identifying the guest/host uniquely within the
>>>> guest. Adding following properties to the guest root node.
>>>>
>>>> vm,uuid - uuid of the guest
>>>> host-model - Host model number
>>>> host-serial - Host machine serial number
>>>> hypervisor type - Tells its "kvm"
>>>>
>>>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>>>>
>>>> ---
>>>> v4: make uuid as human readable
>>>> v3: rebase to ppcnext
>>>> v2: indentation fixes
>>>> ---
>>>> hw/ppc/spapr.c | 25 +++++++++++++++++++++++++
>>>> target-ppc/kvm.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
>>>> target-ppc/kvm_ppc.h | 12 ++++++++++++
>>>> 3 files changed, 80 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>> index 077ad2d..485ea66 100644
>>>> --- a/hw/ppc/spapr.c
>>>> +++ b/hw/ppc/spapr.c
>>>> @@ -318,6 +318,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>>>> QemuOpts *opts = qemu_opts_find(qemu_find_opts("smp-opts"), NULL);
>>>> unsigned sockets = opts ? qemu_opt_get_number(opts, "sockets", 0) : 0;
>>>> uint32_t cpus_per_socket = sockets ? (smp_cpus / sockets) : 1;
>>>> + char char_buf[512];
>>> Can't you just return callee allocated, caller free'd memory?
>> Tried doing it more in line of read_cpuinfo in target-ppc/kvm.c
>> I could do it either ways.
>
> Yeah, feel free to convert that one too if you like ;).
>
>>
>>>>
>>>> add_str(hypertas, "hcall-pft");
>>>> add_str(hypertas, "hcall-term");
>>>> @@ -347,6 +348,30 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>>>> _FDT((fdt_property_string(fdt, "model", "IBM pSeries (emulated by qemu)")));
>>>> _FDT((fdt_property_string(fdt, "compatible", "qemu,pseries")));
>>>>
>>>> + if (kvm_enabled()) {
>>>> + _FDT((fdt_property_string(fdt, "hypervisor", "kvm")));
>>>> + }
>>>> +
>>>> + /*
>>>> + * Add info to guest to indentify which host is it being run on
>>>> + * and what is the uuid of the guest
>>>> + */
>>>> + memset(char_buf, 0, sizeof(char_buf));
>>>> + if (!kvmppc_get_host_model(char_buf, sizeof(char_buf))) {
>>>> + _FDT((fdt_property_string(fdt, "host-model", char_buf)));
>>>> + memset(char_buf, 0, sizeof(char_buf));
>>>> + }
>>>> + if (!kvmppc_get_host_serial(char_buf, sizeof(char_buf))) {
>>>> + _FDT((fdt_property_string(fdt, "host-serial", char_buf)));
>>>> + }
>>> Please be aware that all of the above is bogus when you start thinking
>>> about live migration.
>> Yes, there are tools that look at these. Is there a way to update these
>> on migration?
>
> As Ben already mentioned ;).
>
>>
>>>> +
>>>> + snprintf(char_buf, 37, UUID_FMT, qemu_uuid[0], qemu_uuid[1],
>>> g_strdup_printf()
>> Ok.
>>
>>>> + qemu_uuid[2], qemu_uuid[3], qemu_uuid[4], qemu_uuid[5],
>>>> + qemu_uuid[6], qemu_uuid[7], qemu_uuid[8], qemu_uuid[9],
>>>> + qemu_uuid[10], qemu_uuid[11], qemu_uuid[12], qemu_uuid[13],
>>>> + qemu_uuid[14], qemu_uuid[15]);
>>>> + _FDT((fdt_property_string(fdt, "vm,uuid", char_buf)));
>>>> +
>>>> _FDT((fdt_property_cell(fdt, "#address-cells", 0x2)));
>>>> _FDT((fdt_property_cell(fdt, "#size-cells", 0x2)));
>>>>
>>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>>>> index 2d87108..25091f8 100644
>>>> --- a/target-ppc/kvm.c
>>>> +++ b/target-ppc/kvm.c
>>>> @@ -1369,7 +1369,7 @@ static int read_cpuinfo(const char *field, char *value, int len)
>>>> }
>>>>
>>>> do {
>>>> - if(!fgets(line, sizeof(line), f)) {
>>>> + if (!fgets(line, sizeof(line), f)) {
>>>> break;
>>>> }
>>>> if (!strncmp(line, field, field_len)) {
>>>> @@ -1404,6 +1404,48 @@ uint32_t kvmppc_get_tbfreq(void)
>>>> return retval;
>>>> }
>>>>
>>>> +int32_t kvmppc_get_host_serial(char *value, int len)
>>>> +{
>>>> + FILE *f;
>>>> + int ret = -1;
>>>> + char line[512];
>>>> +
>>>> + memset(line, 0, sizeof(line));
>>>> + f = fopen("/proc/device-tree/system-id", "r");
>>>> + if (!f) {
>>>> + return ret;
>>>> + }
>>>> +
>>>> + if (fgets(line, sizeof(line), f)) {
>>>> + snprintf(value, len, "IBM,%s", line);
>>> Why IBM,<system-id>?
>> There were userspace tools that looking at lparcfg, and were encoded
>> similarly.
>
> I don't think we own the IBM namespace, so I find this slightly bogus.
> Also why would a host machine have to be made by IBM?
You have a valid point, i will remove the "IBM" prefix :-)
>
>>
>>>> + ret = 0;
>>>> + }
>>>> + fclose(f);
>>>> +
>>>> + return ret;
>>> I think it makes sense to extract the "read a full file into a buffer"
>>> logic into a separate function. For bonus points, find a glib function
>>> that already does it and use that ;).
>> Let me search.
>>
>>>> +}
>>>> +
>>>> +int32_t kvmppc_get_host_model(char *value, int len)
>>>> +{
>>>> + FILE *f;
>>>> + int ret = -1;
>>>> + char line[512];
>>>> +
>>>> + memset(line, 0, sizeof(line));
>>>> + f = fopen("/proc/device-tree/model", "r");
>>>> + if (!f) {
>>>> + return ret;
>>>> + }
>>>> +
>>>> + if (fgets(line, sizeof(line), f)) {
>>>> + snprintf(value, len, "IBM,%s", line);
>>> Same here - wouldn't this be IBM,IBM,foo?
>> No, it will be IBM,<model>
>
> Hrm, I just tried to compare this with a pHyp system and can't seem to
> find any /proc/device-tree/host* files. What am I missing?
You wont, they arent there in pHyp. It was being populated in
/proc/ppc64/lparcfg. For example /model which we encode as :
_FDT((fdt_property_string(fdt, "model", "IBM pSeries (emulated by qemu)")));
pHyp would have put it as host machine model. From above we digressed.
I thought of having a mechanism that is more generic to get these from
device tree directly.
Regards,
Nikunj
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-07-08 11:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-08 5:00 [Qemu-devel] [PATCH v4] spapr: add uuid/host details to device tree Nikunj A Dadhania
2014-07-08 10:49 ` Alexander Graf
2014-07-08 10:55 ` [Qemu-devel] [Qemu-ppc] " Benjamin Herrenschmidt
2014-07-08 11:04 ` [Qemu-devel] " Nikunj A Dadhania
2014-07-08 11:11 ` Alexander Graf
2014-07-08 11:26 ` Nikunj A Dadhania
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).