| Message ID | 1387227072-21965-4-git-send-email-mjrosato@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 16.12.2013, at 21:51, Matthew Rosato <mjrosato@linux.vnet.ibm.com> wrote: > When machine=...,standby-mem={size} has been specified, convert the value > to bytes and store it for use. > > Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com> > --- > target-s390x/kvm.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c > index 02ac4ba..d4081f4 100644 > --- a/target-s390x/kvm.c > +++ b/target-s390x/kvm.c > @@ -97,11 +97,27 @@ static void *legacy_s390_alloc(size_t size); > > int kvm_arch_init(KVMState *s) > { > + int64_t value; > + > cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS); > if (!kvm_check_extension(s, KVM_CAP_S390_GMAP) > || !kvm_check_extension(s, KVM_CAP_S390_COW)) { > phys_mem_set_alloc(legacy_s390_alloc); > } > + > + value = qemu_opt_get_size(qemu_get_machine_opts(), "standby-mem", -1); > + > + if (value < 0) { > + fprintf(stderr, "qemu: invalid standby-mem size:%"PRId64"\n", value); > + exit(1); > + } > + > + if (value != (int64_t)(ram_addr_t)value) { > + fprintf(stderr, "qemu: standby size too large\n"); > + exit(1); > + } > + standby_mem_size = value * 1024 * 1024; I would hope qemu_opt_get_size() returns a value in bytes. Why multiply it here? Alex
On 12/16/2013 04:25 PM, Alexander Graf wrote: > > On 16.12.2013, at 21:51, Matthew Rosato <mjrosato@linux.vnet.ibm.com> wrote: > >> When machine=...,standby-mem={size} has been specified, convert the value >> to bytes and store it for use. >> >> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com> >> --- >> target-s390x/kvm.c | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> >> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c >> index 02ac4ba..d4081f4 100644 >> --- a/target-s390x/kvm.c >> +++ b/target-s390x/kvm.c >> @@ -97,11 +97,27 @@ static void *legacy_s390_alloc(size_t size); >> >> int kvm_arch_init(KVMState *s) >> { >> + int64_t value; >> + >> cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS); >> if (!kvm_check_extension(s, KVM_CAP_S390_GMAP) >> || !kvm_check_extension(s, KVM_CAP_S390_COW)) { >> phys_mem_set_alloc(legacy_s390_alloc); >> } >> + >> + value = qemu_opt_get_size(qemu_get_machine_opts(), "standby-mem", -1); >> + >> + if (value < 0) { >> + fprintf(stderr, "qemu: invalid standby-mem size:%"PRId64"\n", value); >> + exit(1); >> + } >> + >> + if (value != (int64_t)(ram_addr_t)value) { >> + fprintf(stderr, "qemu: standby size too large\n"); >> + exit(1); >> + } >> + standby_mem_size = value * 1024 * 1024; > > I would hope qemu_opt_get_size() returns a value in bytes. Why multiply it here? > It's actually in megabytes, and this is converting to bytes -- Based on your comment, it sounds like qemu_opt_get_size should always be returning a byte value. FWIW, if I adopt Paolo's comments re: patch 1, this behavior goes away (and the parsing moves out of s390). Either way, I'll adjust for v2. > > Alex > > > >
On 17.12.2013, at 16:58, Matthew Rosato <mjrosato@linux.vnet.ibm.com> wrote: > On 12/16/2013 04:25 PM, Alexander Graf wrote: >> >> On 16.12.2013, at 21:51, Matthew Rosato <mjrosato@linux.vnet.ibm.com> wrote: >> >>> When machine=...,standby-mem={size} has been specified, convert the value >>> to bytes and store it for use. >>> >>> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com> >>> --- >>> target-s390x/kvm.c | 16 ++++++++++++++++ >>> 1 file changed, 16 insertions(+) >>> >>> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c >>> index 02ac4ba..d4081f4 100644 >>> --- a/target-s390x/kvm.c >>> +++ b/target-s390x/kvm.c >>> @@ -97,11 +97,27 @@ static void *legacy_s390_alloc(size_t size); >>> >>> int kvm_arch_init(KVMState *s) >>> { >>> + int64_t value; >>> + >>> cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS); >>> if (!kvm_check_extension(s, KVM_CAP_S390_GMAP) >>> || !kvm_check_extension(s, KVM_CAP_S390_COW)) { >>> phys_mem_set_alloc(legacy_s390_alloc); >>> } >>> + >>> + value = qemu_opt_get_size(qemu_get_machine_opts(), "standby-mem", -1); >>> + >>> + if (value < 0) { >>> + fprintf(stderr, "qemu: invalid standby-mem size:%"PRId64"\n", value); >>> + exit(1); >>> + } >>> + >>> + if (value != (int64_t)(ram_addr_t)value) { >>> + fprintf(stderr, "qemu: standby size too large\n"); >>> + exit(1); >>> + } >>> + standby_mem_size = value * 1024 * 1024; >> >> I would hope qemu_opt_get_size() returns a value in bytes. Why multiply it here? >> > > It's actually in megabytes, and this is converting to bytes -- Based on > your comment, it sounds like qemu_opt_get_size should always be > returning a byte value. At least I don't see code that converts it into MB. After all, you should be able to say size=1M which with your code probably ends up as 1T, no? > FWIW, if I adopt Paolo's comments re: patch 1, this behavior goes away > (and the parsing moves out of s390). Either way, I'll adjust for v2. Ok :). Alex
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c index 02ac4ba..d4081f4 100644 --- a/target-s390x/kvm.c +++ b/target-s390x/kvm.c @@ -97,11 +97,27 @@ static void *legacy_s390_alloc(size_t size); int kvm_arch_init(KVMState *s) { + int64_t value; + cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS); if (!kvm_check_extension(s, KVM_CAP_S390_GMAP) || !kvm_check_extension(s, KVM_CAP_S390_COW)) { phys_mem_set_alloc(legacy_s390_alloc); } + + value = qemu_opt_get_size(qemu_get_machine_opts(), "standby-mem", -1); + + if (value < 0) { + fprintf(stderr, "qemu: invalid standby-mem size:%"PRId64"\n", value); + exit(1); + } + + if (value != (int64_t)(ram_addr_t)value) { + fprintf(stderr, "qemu: standby size too large\n"); + exit(1); + } + standby_mem_size = value * 1024 * 1024; + return 0; }
When machine=...,standby-mem={size} has been specified, convert the value to bytes and store it for use. Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com> --- target-s390x/kvm.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)