[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH v4] spapr: add uuid/host details to device tree
From: |
Nikunj A Dadhania |
Subject: |
Re: [Qemu-ppc] [PATCH v4] spapr: add uuid/host details to device tree |
Date: |
Tue, 08 Jul 2014 16:56:57 +0530 |
User-agent: |
Notmuch/0.17+27~gae47d61 (http://notmuchmail.org) Emacs/24.3.1 (x86_64-redhat-linux-gnu) |
Alexander Graf <address@hidden> writes:
> On 08.07.14 13:04, Nikunj A Dadhania wrote:
>> Alexander Graf <address@hidden> 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 <address@hidden>
>>>>
>>>> ---
>>>> 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