| Message ID | 1405657877-12353-1-git-send-email-cyrilbur@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 07/17/2014 10:31 PM, cyrilbur@gmail.com wrote: > It may prove useful know which Linux distribution version the host machine > is running when an issue in the guest arises but a user cannot access > the host. > > Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> > --- > hw/ppc/spapr.c | 8 +++++++ > target-ppc/kvm.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > target-ppc/kvm_ppc.h | 6 +++++ > 3 files changed, 76 insertions(+) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 6b48a26..391d47a 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -375,6 +375,14 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base, > _FDT((fdt_property_string(fdt, "vm,uuid", buf))); > g_free(buf); > > + /* > + * Add info to the guest FDT to tell it what linux the host is > + */ > + if (kvmppc_get_linux_host(&buf)) { > + _FDT((fdt_property_string(fdt, "linux,host", buf))); > + g_free(buf); > + } Ouch. What does this do for migration? By exposing it to the guest, you have made it part of the guest ABI, and now you have limited yourself to migrate only when the destination host is identical to the source if you don't want to risk breaking the guest. Without this, it seems feasible to migrate from a machine on an older host to another machine on a newer host.
On 18.07.14 13:59, Eric Blake wrote: > On 07/17/2014 10:31 PM, cyrilbur@gmail.com wrote: >> It may prove useful know which Linux distribution version the host machine >> is running when an issue in the guest arises but a user cannot access >> the host. >> >> Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> >> --- >> hw/ppc/spapr.c | 8 +++++++ >> target-ppc/kvm.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++ >> target-ppc/kvm_ppc.h | 6 +++++ >> 3 files changed, 76 insertions(+) >> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 6b48a26..391d47a 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -375,6 +375,14 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base, >> _FDT((fdt_property_string(fdt, "vm,uuid", buf))); >> g_free(buf); >> >> + /* >> + * Add info to the guest FDT to tell it what linux the host is >> + */ >> + if (kvmppc_get_linux_host(&buf)) { >> + _FDT((fdt_property_string(fdt, "linux,host", buf))); >> + g_free(buf); >> + } > Ouch. What does this do for migration? By exposing it to the guest, > you have made it part of the guest ABI, and now you have limited > yourself to migrate only when the destination host is identical to the > source if you don't want to risk breaking the guest. Without this, it > seems feasible to migrate from a machine on an older host to another > machine on a newer host. sPAPR has a mechanism to inject a "post-migration" interrupt into the guest and provide a device tree diff. That way we can change these host specific values on the fly after migration happened. Maybe it makes sense to hold off these host specific device tree chunks until after we have that mechanism implemented though. Alex
On 18/07/14 22:01, Alexander Graf wrote: > > On 18.07.14 13:59, Eric Blake wrote: >> On 07/17/2014 10:31 PM, cyrilbur@gmail.com wrote: >>> It may prove useful know which Linux distribution version the host >>> machine >>> is running when an issue in the guest arises but a user cannot access >>> the host. >>> >>> Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> >>> --- >>> hw/ppc/spapr.c | 8 +++++++ >>> target-ppc/kvm.c | 62 >>> ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> target-ppc/kvm_ppc.h | 6 +++++ >>> 3 files changed, 76 insertions(+) >>> >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >>> index 6b48a26..391d47a 100644 >>> --- a/hw/ppc/spapr.c >>> +++ b/hw/ppc/spapr.c >>> @@ -375,6 +375,14 @@ static void *spapr_create_fdt_skel(hwaddr >>> initrd_base, >>> _FDT((fdt_property_string(fdt, "vm,uuid", buf))); >>> g_free(buf); >>> + /* >>> + * Add info to the guest FDT to tell it what linux the host is >>> + */ >>> + if (kvmppc_get_linux_host(&buf)) { >>> + _FDT((fdt_property_string(fdt, "linux,host", buf))); >>> + g_free(buf); >>> + } >> Ouch. What does this do for migration? By exposing it to the guest, >> you have made it part of the guest ABI, and now you have limited >> yourself to migrate only when the destination host is identical to the >> source if you don't want to risk breaking the guest. Without this, it >> seems feasible to migrate from a machine on an older host to another >> machine on a newer host. > > sPAPR has a mechanism to inject a "post-migration" interrupt into the > guest and provide a device tree diff. That way we can change these > host specific values on the fly after migration happened. > > Maybe it makes sense to hold off these host specific device tree > chunks until after we have that mechanism implemented though. Indeed and the ability to do this is in the kernel but it had not been implemented in qemu yet. Probably not worth doing just for this patch but having this does open the doors to quite a few other things, I'll investigate. Cyril > > Alex >
On 18.07.14 06:31, cyrilbur@gmail.com wrote: > It may prove useful know which Linux distribution version the host machine > is running when an issue in the guest arises but a user cannot access > the host. > > Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> > --- > hw/ppc/spapr.c | 8 +++++++ > target-ppc/kvm.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > target-ppc/kvm_ppc.h | 6 +++++ > 3 files changed, 76 insertions(+) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 6b48a26..391d47a 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -375,6 +375,14 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base, > _FDT((fdt_property_string(fdt, "vm,uuid", buf))); > g_free(buf); > > + /* > + * Add info to the guest FDT to tell it what linux the host is > + */ > + if (kvmppc_get_linux_host(&buf)) { > + _FDT((fdt_property_string(fdt, "linux,host", buf))); Is this even specified in sPAPR? > + g_free(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 8c9e79c..95e0970 100644 > --- a/target-ppc/kvm.c > +++ b/target-ppc/kvm.c > @@ -1415,6 +1415,68 @@ bool kvmppc_get_host_model(char **value) > return g_file_get_contents("/proc/device-tree/model", value, NULL, NULL); > } > > +bool kvmppc_get_linux_host(char **value) > +{ > + FILE *f; > + int i; > + char line[512]; > + const char *names[] = {"NAME", "VERSION", "BUILD_ID"}; > + bool names_found[ARRAY_SIZE(names)] = { 0 }; > + GString *output = NULL; > + f = fopen("/etc/os-release", "r"); A few comments: 1) Why would anyone care? 2) I'm not sure KVM is the right decision maker on whether we want this exposed or not. After all, the files you read here are available on an x86 host just as well 3) Use glib functions to read files Alex
On 07/24/2014 11:15 PM, Alexander Graf wrote: > > On 18.07.14 06:31, cyrilbur@gmail.com wrote: >> It may prove useful know which Linux distribution version the host machine >> is running when an issue in the guest arises but a user cannot access >> the host. >> >> Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> >> --- >> hw/ppc/spapr.c | 8 +++++++ >> target-ppc/kvm.c | 62 >> ++++++++++++++++++++++++++++++++++++++++++++++++++++ >> target-ppc/kvm_ppc.h | 6 +++++ >> 3 files changed, 76 insertions(+) >> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 6b48a26..391d47a 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -375,6 +375,14 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base, >> _FDT((fdt_property_string(fdt, "vm,uuid", buf))); >> g_free(buf); >> + /* >> + * Add info to the guest FDT to tell it what linux the host is >> + */ >> + if (kvmppc_get_linux_host(&buf)) { >> + _FDT((fdt_property_string(fdt, "linux,host", buf))); > > Is this even specified in sPAPR? PAPR does not know about any "linux,xxx" properties. > >> + g_free(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 8c9e79c..95e0970 100644 >> --- a/target-ppc/kvm.c >> +++ b/target-ppc/kvm.c >> @@ -1415,6 +1415,68 @@ bool kvmppc_get_host_model(char **value) >> return g_file_get_contents("/proc/device-tree/model", value, NULL, >> NULL); >> } >> +bool kvmppc_get_linux_host(char **value) >> +{ >> + FILE *f; >> + int i; >> + char line[512]; >> + const char *names[] = {"NAME", "VERSION", "BUILD_ID"}; >> + bool names_found[ARRAY_SIZE(names)] = { 0 }; >> + GString *output = NULL; >> + f = fopen("/etc/os-release", "r"); > > A few comments: > > 1) Why would anyone care? Useful debug info when the host is not reachable. > 2) I'm not sure KVM is the right decision maker on whether we want this > exposed or not. Good point, there is no reason not to show this info in TCG. > After all, the files you read here are available on an x86 > host just as well Does qemu-x86 has any way to pass information like this to the guest? > 3) Use glib functions to read files > > > Alex > >
> Am 28.07.2014 um 08:47 schrieb Alexey Kardashevskiy <aik@ozlabs.ru>: > >> On 07/24/2014 11:15 PM, Alexander Graf wrote: >> >>> On 18.07.14 06:31, cyrilbur@gmail.com wrote: >>> It may prove useful know which Linux distribution version the host machine >>> is running when an issue in the guest arises but a user cannot access >>> the host. >>> >>> Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> >>> --- >>> hw/ppc/spapr.c | 8 +++++++ >>> target-ppc/kvm.c | 62 >>> ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> target-ppc/kvm_ppc.h | 6 +++++ >>> 3 files changed, 76 insertions(+) >>> >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >>> index 6b48a26..391d47a 100644 >>> --- a/hw/ppc/spapr.c >>> +++ b/hw/ppc/spapr.c >>> @@ -375,6 +375,14 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base, >>> _FDT((fdt_property_string(fdt, "vm,uuid", buf))); >>> g_free(buf); >>> + /* >>> + * Add info to the guest FDT to tell it what linux the host is >>> + */ >>> + if (kvmppc_get_linux_host(&buf)) { >>> + _FDT((fdt_property_string(fdt, "linux,host", buf))); >> >> Is this even specified in sPAPR? > > > PAPR does not know about any "linux,xxx" properties. > >> >>> + g_free(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 8c9e79c..95e0970 100644 >>> --- a/target-ppc/kvm.c >>> +++ b/target-ppc/kvm.c >>> @@ -1415,6 +1415,68 @@ bool kvmppc_get_host_model(char **value) >>> return g_file_get_contents("/proc/device-tree/model", value, NULL, >>> NULL); >>> } >>> +bool kvmppc_get_linux_host(char **value) >>> +{ >>> + FILE *f; >>> + int i; >>> + char line[512]; >>> + const char *names[] = {"NAME", "VERSION", "BUILD_ID"}; >>> + bool names_found[ARRAY_SIZE(names)] = { 0 }; >>> + GString *output = NULL; >>> + f = fopen("/etc/os-release", "r"); >> >> A few comments: >> >> 1) Why would anyone care? > > > Useful debug info when the host is not reachable. > > >> 2) I'm not sure KVM is the right decision maker on whether we want this >> exposed or not. > > Good point, there is no reason not to show this info in TCG. > >> After all, the files you read here are available on an x86 >> host just as well > > Does qemu-x86 has any way to pass information like this to the guest? Yes and no. It has fw_cfg where we could put it and it has DSST generation where we could also put it I guess. Just like here, there's no standardized way to expose it though. Alex > > >> 3) Use glib functions to read files >> >> >> Alex > > > -- > Alexey
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 6b48a26..391d47a 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -375,6 +375,14 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base, _FDT((fdt_property_string(fdt, "vm,uuid", buf))); g_free(buf); + /* + * Add info to the guest FDT to tell it what linux the host is + */ + if (kvmppc_get_linux_host(&buf)) { + _FDT((fdt_property_string(fdt, "linux,host", buf))); + g_free(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 8c9e79c..95e0970 100644 --- a/target-ppc/kvm.c +++ b/target-ppc/kvm.c @@ -1415,6 +1415,68 @@ bool kvmppc_get_host_model(char **value) return g_file_get_contents("/proc/device-tree/model", value, NULL, NULL); } +bool kvmppc_get_linux_host(char **value) +{ + FILE *f; + int i; + char line[512]; + const char *names[] = {"NAME", "VERSION", "BUILD_ID"}; + bool names_found[ARRAY_SIZE(names)] = { 0 }; + GString *output = NULL; + f = fopen("/etc/os-release", "r"); + if (f) { + output = g_string_new(NULL); + while (fgets(line, sizeof(line), f) && output) { + if (!strchr(line, '=')) { + continue; + } + + for (i = 0; i < ARRAY_SIZE(names); i++) { + char *start; + int name_len = strlen(names[i]); + + if (strncmp(line, names[i], name_len)) { + continue; + } + if (names_found[i]) { + break; + } + + names_found[i] = true; + start = line + name_len + 1; + if (*start == '"') { + start++; + } + + char *end = start + strlen(start) - 1; + while (end >= start && strchr("\"\n\t ", *end)) { + end--; + } + end[1] = '\0'; + + g_string_append(output, start); + } + } + fclose(f); + } else { + f = fopen("/etc/issue", "r"); + if (!f) { + return false; + } + + fgets(line, sizeof(line), f); + fclose(f); + output = g_string_new(line); + } + + if (output) { + *value = g_string_free(output, false); + return true; + } + + return false; +} + /* 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 2e0224c..92a71f7 100644 --- a/target-ppc/kvm_ppc.h +++ b/target-ppc/kvm_ppc.h @@ -21,6 +21,7 @@ uint32_t kvmppc_get_vmx(void); uint32_t kvmppc_get_dfp(void); bool kvmppc_get_host_model(char **buf); bool kvmppc_get_host_serial(char **buf); +bool kvmppc_get_linux_host(char **buf); 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); @@ -72,6 +73,11 @@ static inline bool kvmppc_get_host_serial(char **buf) return false; } +static inline bool kvmppc_get_linux_host(char **buf) +{ + return false; +} + static inline uint64_t kvmppc_get_clockfreq(void) { return 0;
It may prove useful know which Linux distribution version the host machine is running when an issue in the guest arises but a user cannot access the host. Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> --- hw/ppc/spapr.c | 8 +++++++ target-ppc/kvm.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++ target-ppc/kvm_ppc.h | 6 +++++ 3 files changed, 76 insertions(+)