From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37840) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X4TJS-0000gg-0F for qemu-devel@nongnu.org; Tue, 08 Jul 2014 07:12:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X4TJK-0001N2-ST for qemu-devel@nongnu.org; Tue, 08 Jul 2014 07:11:53 -0400 Message-ID: <53BBD1F1.1050500@suse.de> Date: Tue, 08 Jul 2014 13:11:45 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1404795606-25973-1-git-send-email-nikunj@linux.vnet.ibm.com> <53BBCC9D.5070000@suse.de> <871ttwjemi.fsf@linux.vnet.ibm.com> In-Reply-To: <871ttwjemi.fsf@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4] spapr: add uuid/host details to device tree List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikunj A Dadhania , qemu-devel@nongnu.org Cc: aik@ozlabs.ru, qemu-ppc@nongnu.org On 08.07.14 13:04, Nikunj A Dadhania wrote: > Alexander Graf 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 >>> >>> --- >>> 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,? > 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, 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