qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/13] Misc fixes for QEMU 3.0.0-rc1
@ 2018-07-17 15:06 Paolo Bonzini
  2018-07-17 15:06 ` [Qemu-devel] [PULL 01/13] dump: add kernel_gs_base to QEMU CPU state Paolo Bonzini
                   ` (13 more replies)
  0 siblings, 14 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-17 15:06 UTC (permalink / raw)
  To: qemu-devel

The following changes since commit 9277d81f5c2c6f4d0b5e47c8476eb7ee7e5c0beb:

  docs: Grammar and spelling fixes (2018-07-13 10:16:04 +0100)

are available in the git repository at:

  git://github.com/bonzini/qemu.git 

for you to fetch changes up to dfaa7d50b0f72060764096ffcae4a0c06ce24f9b:

  Document command line options with single dash (2018-07-17 16:24:50 +0200)

----------------------------------------------------------------
Bug fixes.

----------------------------------------------------------------
BALATON Zoltan (1):
      Document command line options with single dash

Calvin Lee (1):
      PC Chipset: Improve serial divisor calculation

Daniel P. Berrangé (3):
      i386: fix regression parsing multiboot initrd modules
      i386: only parse the initrd_filename once for multiboot modules
      opts: remove redundant check for NULL parameter

Emanuele Giuseppe Esposito (1):
      vhost-user-test: added proper TestServer *dest initialization in test_migrate()

Marc-André Lureau (1):
      hw/char/serial: retry write if EAGAIN

Roman Kagan (2):
      hyperv: rename vcpu_id to vp_index
      hyperv: ensure VP index equal to QEMU cpu_index

Stefan Hajnoczi (2):
      qdev: add HotplugHandler->post_plug() callback
      virtio-scsi: fix hotplug ->reset() vs event race

Stefan Weil (1):
      accel: Fix typo and grammar in comment

Viktor Prutyanov (1):
      dump: add kernel_gs_base to QEMU CPU state

 accel/tcg/translate-all.c |  2 +-
 hw/char/serial.c          | 46 +++++++++++++++++++++++++++-------------------
 hw/core/hotplug.c         | 10 ++++++++++
 hw/core/qdev.c            |  4 ++++
 hw/i386/multiboot.c       | 35 ++++++++++++++++-------------------
 hw/i386/pc.c              |  5 +++++
 hw/misc/hyperv_testdev.c  | 16 ++++++++--------
 hw/scsi/virtio-scsi.c     | 11 ++++++++++-
 include/hw/hotplug.h      | 11 +++++++++++
 qemu-options.hx           |  8 ++++----
 target/i386/arch_dump.c   | 10 ++++++++++
 target/i386/hyperv.c      | 16 +++++++++++++---
 target/i386/hyperv.h      |  7 +++++--
 target/i386/kvm-stub.c    |  5 +++++
 target/i386/kvm.c         | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 target/i386/kvm_i386.h    |  2 ++
 tests/vhost-user-test.c   |  3 +++
 util/qemu-option.c        |  8 +++-----
 18 files changed, 184 insertions(+), 62 deletions(-)
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PULL 01/13] dump: add kernel_gs_base to QEMU CPU state
  2018-07-17 15:06 [Qemu-devel] [PULL 00/13] Misc fixes for QEMU 3.0.0-rc1 Paolo Bonzini
@ 2018-07-17 15:06 ` Paolo Bonzini
  2018-07-17 15:06 ` [Qemu-devel] [PULL 02/13] accel: Fix typo and grammar in comment Paolo Bonzini
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-17 15:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Viktor Prutyanov

From: Viktor Prutyanov <viktor.prutyanov@virtuozzo.com>

This patch adds field with content of KERNEL_GS_BASE MSR to QEMU note in
ELF dump.

On Windows, if all vCPUs are running usermode tasks at the time the dump is
created, this can be helpful in the discovery of guest system structures
during conversion ELF dump to MEMORY.DMP dump.

Signed-off-by: Viktor Prutyanov <viktor.prutyanov@virtuozzo.com>
Message-Id: <20180714123000.11326-1-viktor.prutyanov@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/arch_dump.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/target/i386/arch_dump.c b/target/i386/arch_dump.c
index 35b55fc..004141f 100644
--- a/target/i386/arch_dump.c
+++ b/target/i386/arch_dump.c
@@ -258,6 +258,12 @@ struct QEMUCPUState {
     QEMUCPUSegment cs, ds, es, fs, gs, ss;
     QEMUCPUSegment ldt, tr, gdt, idt;
     uint64_t cr[5];
+    /*
+     * Fields below are optional and are being added at the end without
+     * changing the version. External tools may identify their presence
+     * by checking 'size' field.
+     */
+    uint64_t kernel_gs_base;
 };
 
 typedef struct QEMUCPUState QEMUCPUState;
@@ -315,6 +321,10 @@ static void qemu_get_cpustate(QEMUCPUState *s, CPUX86State *env)
     s->cr[2] = env->cr[2];
     s->cr[3] = env->cr[3];
     s->cr[4] = env->cr[4];
+
+#ifdef TARGET_X86_64
+    s->kernel_gs_base = env->kernelgsbase;
+#endif
 }
 
 static inline int cpu_write_qemu_note(WriteCoreDumpFunction f,
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PULL 02/13] accel: Fix typo and grammar in comment
  2018-07-17 15:06 [Qemu-devel] [PULL 00/13] Misc fixes for QEMU 3.0.0-rc1 Paolo Bonzini
  2018-07-17 15:06 ` [Qemu-devel] [PULL 01/13] dump: add kernel_gs_base to QEMU CPU state Paolo Bonzini
@ 2018-07-17 15:06 ` Paolo Bonzini
  2018-07-17 15:06 ` [Qemu-devel] [PULL 03/13] hyperv: rename vcpu_id to vp_index Paolo Bonzini
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-17 15:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Weil

From: Stefan Weil <sw@weilnetz.de>

The typo was found by codespell.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
Message-Id: <20180712194454.26765-1-sw@weilnetz.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 accel/tcg/translate-all.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 49d77fa..1571987 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1795,7 +1795,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
     tb->jmp_dest[0] = (uintptr_t)NULL;
     tb->jmp_dest[1] = (uintptr_t)NULL;
 
-    /* init original jump addresses wich has been set during tcg_gen_code() */
+    /* init original jump addresses which have been set during tcg_gen_code() */
     if (tb->jmp_reset_offset[0] != TB_JMP_RESET_OFFSET_INVALID) {
         tb_reset_jump(tb, 0);
     }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PULL 03/13] hyperv: rename vcpu_id to vp_index
  2018-07-17 15:06 [Qemu-devel] [PULL 00/13] Misc fixes for QEMU 3.0.0-rc1 Paolo Bonzini
  2018-07-17 15:06 ` [Qemu-devel] [PULL 01/13] dump: add kernel_gs_base to QEMU CPU state Paolo Bonzini
  2018-07-17 15:06 ` [Qemu-devel] [PULL 02/13] accel: Fix typo and grammar in comment Paolo Bonzini
@ 2018-07-17 15:06 ` Paolo Bonzini
  2018-07-17 15:06 ` [Qemu-devel] [PULL 04/13] hyperv: ensure VP index equal to QEMU cpu_index Paolo Bonzini
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-17 15:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Roman Kagan

From: Roman Kagan <rkagan@virtuozzo.com>

In Hyper-V-related code, vCPUs are identified by their VP (virtual
processor) index.  Since it's customary for "vcpu_id" in QEMU to mean
APIC id, rename the respective variables to "vp_index" to make the
distinction clear.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
Message-Id: <20180702134156.13404-2-rkagan@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/misc/hyperv_testdev.c | 16 ++++++++--------
 target/i386/hyperv.c     |  6 +++---
 target/i386/hyperv.h     |  4 ++--
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/hw/misc/hyperv_testdev.c b/hw/misc/hyperv_testdev.c
index dbd7cdd..bf6bbfa 100644
--- a/hw/misc/hyperv_testdev.c
+++ b/hw/misc/hyperv_testdev.c
@@ -57,7 +57,7 @@ static void free_sint_route_index(HypervTestDev *dev, int i)
     dev->sint_route[i] = NULL;
 }
 
-static int find_sint_route_index(HypervTestDev *dev, uint32_t vcpu_id,
+static int find_sint_route_index(HypervTestDev *dev, uint32_t vp_index,
                                  uint32_t sint)
 {
     HvSintRoute *sint_route;
@@ -65,7 +65,7 @@ static int find_sint_route_index(HypervTestDev *dev, uint32_t vcpu_id,
 
     for (i = 0; i < ARRAY_SIZE(dev->sint_route); i++) {
         sint_route = dev->sint_route[i];
-        if (sint_route && sint_route->vcpu_id == vcpu_id &&
+        if (sint_route && sint_route->vp_index == vp_index &&
             sint_route->sint == sint) {
             return i;
         }
@@ -74,7 +74,7 @@ static int find_sint_route_index(HypervTestDev *dev, uint32_t vcpu_id,
 }
 
 static void hv_synic_test_dev_control(HypervTestDev *dev, uint32_t ctl,
-                                      uint32_t vcpu_id, uint32_t sint)
+                                      uint32_t vp_index, uint32_t sint)
 {
     int i;
     HvSintRoute *sint_route;
@@ -83,19 +83,19 @@ static void hv_synic_test_dev_control(HypervTestDev *dev, uint32_t ctl,
     case HV_TEST_DEV_SINT_ROUTE_CREATE:
         i = alloc_sint_route_index(dev);
         assert(i >= 0);
-        sint_route = kvm_hv_sint_route_create(vcpu_id, sint, NULL);
+        sint_route = kvm_hv_sint_route_create(vp_index, sint, NULL);
         assert(sint_route);
         dev->sint_route[i] = sint_route;
         break;
     case HV_TEST_DEV_SINT_ROUTE_DESTROY:
-        i = find_sint_route_index(dev, vcpu_id, sint);
+        i = find_sint_route_index(dev, vp_index, sint);
         assert(i >= 0);
         sint_route = dev->sint_route[i];
         kvm_hv_sint_route_destroy(sint_route);
         free_sint_route_index(dev, i);
         break;
     case HV_TEST_DEV_SINT_ROUTE_SET_SINT:
-        i = find_sint_route_index(dev, vcpu_id, sint);
+        i = find_sint_route_index(dev, vp_index, sint);
         assert(i >= 0);
         sint_route = dev->sint_route[i];
         kvm_hv_sint_route_set_sint(sint_route);
@@ -117,8 +117,8 @@ static void hv_test_dev_control(void *opaque, hwaddr addr, uint64_t data,
     case HV_TEST_DEV_SINT_ROUTE_DESTROY:
     case HV_TEST_DEV_SINT_ROUTE_SET_SINT: {
         uint8_t sint = data & 0xFF;
-        uint8_t vcpu_id = (data >> 8ULL) & 0xFF;
-        hv_synic_test_dev_control(dev, ctl, vcpu_id, sint);
+        uint8_t vp_index = (data >> 8ULL) & 0xFF;
+        hv_synic_test_dev_control(dev, ctl, vp_index, sint);
         break;
     }
     default:
diff --git a/target/i386/hyperv.c b/target/i386/hyperv.c
index a050c9d..7cc0fbb 100644
--- a/target/i386/hyperv.c
+++ b/target/i386/hyperv.c
@@ -72,7 +72,7 @@ static void kvm_hv_sint_ack_handler(EventNotifier *notifier)
     }
 }
 
-HvSintRoute *kvm_hv_sint_route_create(uint32_t vcpu_id, uint32_t sint,
+HvSintRoute *kvm_hv_sint_route_create(uint32_t vp_index, uint32_t sint,
                                       HvSintAckClb sint_ack_clb)
 {
     HvSintRoute *sint_route;
@@ -92,7 +92,7 @@ HvSintRoute *kvm_hv_sint_route_create(uint32_t vcpu_id, uint32_t sint,
     event_notifier_set_handler(&sint_route->sint_ack_notifier,
                                kvm_hv_sint_ack_handler);
 
-    gsi = kvm_irqchip_add_hv_sint_route(kvm_state, vcpu_id, sint);
+    gsi = kvm_irqchip_add_hv_sint_route(kvm_state, vp_index, sint);
     if (gsi < 0) {
         goto err_gsi;
     }
@@ -105,7 +105,7 @@ HvSintRoute *kvm_hv_sint_route_create(uint32_t vcpu_id, uint32_t sint,
     }
     sint_route->gsi = gsi;
     sint_route->sint_ack_clb = sint_ack_clb;
-    sint_route->vcpu_id = vcpu_id;
+    sint_route->vp_index = vp_index;
     sint_route->sint = sint;
 
     return sint_route;
diff --git a/target/i386/hyperv.h b/target/i386/hyperv.h
index 0c3b562..eaf3df3 100644
--- a/target/i386/hyperv.h
+++ b/target/i386/hyperv.h
@@ -23,7 +23,7 @@ typedef void (*HvSintAckClb)(HvSintRoute *sint_route);
 
 struct HvSintRoute {
     uint32_t sint;
-    uint32_t vcpu_id;
+    uint32_t vp_index;
     int gsi;
     EventNotifier sint_set_notifier;
     EventNotifier sint_ack_notifier;
@@ -32,7 +32,7 @@ struct HvSintRoute {
 
 int kvm_hv_handle_exit(X86CPU *cpu, struct kvm_hyperv_exit *exit);
 
-HvSintRoute *kvm_hv_sint_route_create(uint32_t vcpu_id, uint32_t sint,
+HvSintRoute *kvm_hv_sint_route_create(uint32_t vp_index, uint32_t sint,
                                       HvSintAckClb sint_ack_clb);
 
 void kvm_hv_sint_route_destroy(HvSintRoute *sint_route);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PULL 04/13] hyperv: ensure VP index equal to QEMU cpu_index
  2018-07-17 15:06 [Qemu-devel] [PULL 00/13] Misc fixes for QEMU 3.0.0-rc1 Paolo Bonzini
                   ` (2 preceding siblings ...)
  2018-07-17 15:06 ` [Qemu-devel] [PULL 03/13] hyperv: rename vcpu_id to vp_index Paolo Bonzini
@ 2018-07-17 15:06 ` Paolo Bonzini
  2018-07-17 15:06 ` [Qemu-devel] [PULL 05/13] vhost-user-test: added proper TestServer *dest initialization in test_migrate() Paolo Bonzini
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-17 15:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Roman Kagan

From: Roman Kagan <rkagan@virtuozzo.com>

Hyper-V identifies vCPUs by Virtual Processor (VP) index which can be
queried by the guest via HV_X64_MSR_VP_INDEX msr.  It is defined by the
spec as a sequential number which can't exceed the maximum number of
vCPUs per VM.

It has to be owned by QEMU in order to preserve it across migration.

However, the initial implementation in KVM didn't allow to set this
msr, and KVM used its own notion of VP index.  Fortunately, the way
vCPUs are created in QEMU/KVM makes it likely that the KVM value is
equal to QEMU cpu_index.

So choose cpu_index as the value for vp_index, and push that to KVM on
kernels that support setting the msr.  On older ones that don't, query
the kernel value and assert that it's in sync with QEMU.

Besides, since handling errors from vCPU init at hotplug time is
impossible, disable vCPU hotplug.

This patch also introduces accessor functions to encapsulate the mapping
between a vCPU and its vp_index.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
Message-Id: <20180702134156.13404-3-rkagan@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/pc.c           |  5 +++++
 target/i386/hyperv.c   | 10 ++++++++++
 target/i386/hyperv.h   |  3 +++
 target/i386/kvm-stub.c |  5 +++++
 target/i386/kvm.c      | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 target/i386/kvm_i386.h |  2 ++
 6 files changed, 72 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 50d5553..83a4444 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1999,6 +1999,11 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
     }
     cpu->thread_id = topo.smt_id;
 
+    if (cpu->hyperv_vpindex && !kvm_hv_vpindex_settable()) {
+        error_setg(errp, "kernel doesn't allow setting HyperV VP_INDEX");
+        return;
+    }
+
     cs = CPU(cpu);
     cs->cpu_index = idx;
 
diff --git a/target/i386/hyperv.c b/target/i386/hyperv.c
index 7cc0fbb..3065d76 100644
--- a/target/i386/hyperv.c
+++ b/target/i386/hyperv.c
@@ -16,6 +16,16 @@
 #include "hyperv.h"
 #include "hyperv-proto.h"
 
+uint32_t hyperv_vp_index(X86CPU *cpu)
+{
+    return CPU(cpu)->cpu_index;
+}
+
+X86CPU *hyperv_find_vcpu(uint32_t vp_index)
+{
+    return X86_CPU(qemu_get_cpu(vp_index));
+}
+
 int kvm_hv_handle_exit(X86CPU *cpu, struct kvm_hyperv_exit *exit)
 {
     CPUX86State *env = &cpu->env;
diff --git a/target/i386/hyperv.h b/target/i386/hyperv.h
index eaf3df3..00c9b45 100644
--- a/target/i386/hyperv.h
+++ b/target/i386/hyperv.h
@@ -39,4 +39,7 @@ void kvm_hv_sint_route_destroy(HvSintRoute *sint_route);
 
 int kvm_hv_sint_route_set_sint(HvSintRoute *sint_route);
 
+uint32_t hyperv_vp_index(X86CPU *cpu);
+X86CPU *hyperv_find_vcpu(uint32_t vp_index);
+
 #endif
diff --git a/target/i386/kvm-stub.c b/target/i386/kvm-stub.c
index bda4dc2..e7a673e 100644
--- a/target/i386/kvm-stub.c
+++ b/target/i386/kvm-stub.c
@@ -40,3 +40,8 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
     abort();
 }
 #endif
+
+bool kvm_hv_vpindex_settable(void)
+{
+    return false;
+}
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index ebb2d23..9313602 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -85,6 +85,7 @@ static bool has_msr_hv_hypercall;
 static bool has_msr_hv_crash;
 static bool has_msr_hv_reset;
 static bool has_msr_hv_vpindex;
+static bool hv_vpindex_settable;
 static bool has_msr_hv_runtime;
 static bool has_msr_hv_synic;
 static bool has_msr_hv_stimer;
@@ -162,6 +163,11 @@ bool kvm_enable_x2apic(void)
              has_x2apic_api);
 }
 
+bool kvm_hv_vpindex_settable(void)
+{
+    return hv_vpindex_settable;
+}
+
 static int kvm_get_tsc(CPUState *cs)
 {
     X86CPU *cpu = X86_CPU(cs);
@@ -745,6 +751,37 @@ static int hyperv_handle_properties(CPUState *cs)
     return 0;
 }
 
+static int hyperv_init_vcpu(X86CPU *cpu)
+{
+    if (cpu->hyperv_vpindex && !hv_vpindex_settable) {
+        /*
+         * the kernel doesn't support setting vp_index; assert that its value
+         * is in sync
+         */
+        int ret;
+        struct {
+            struct kvm_msrs info;
+            struct kvm_msr_entry entries[1];
+        } msr_data = {
+            .info.nmsrs = 1,
+            .entries[0].index = HV_X64_MSR_VP_INDEX,
+        };
+
+        ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, &msr_data);
+        if (ret < 0) {
+            return ret;
+        }
+        assert(ret == 1);
+
+        if (msr_data.entries[0].data != hyperv_vp_index(cpu)) {
+            error_report("kernel's vp_index != QEMU's vp_index");
+            return -ENXIO;
+        }
+    }
+
+    return 0;
+}
+
 static Error *invtsc_mig_blocker;
 
 #define KVM_MAX_CPUID_ENTRIES  100
@@ -1160,6 +1197,11 @@ int kvm_arch_init_vcpu(CPUState *cs)
         has_msr_tsc_aux = false;
     }
 
+    r = hyperv_init_vcpu(cpu);
+    if (r) {
+        goto fail;
+    }
+
     return 0;
 
  fail:
@@ -1351,6 +1393,8 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
     has_pit_state2 = kvm_check_extension(s, KVM_CAP_PIT_STATE2);
 #endif
 
+    hv_vpindex_settable = kvm_check_extension(s, KVM_CAP_HYPERV_VP_INDEX);
+
     ret = kvm_get_supported_msrs(s);
     if (ret < 0) {
         return ret;
@@ -1900,6 +1944,9 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
         if (has_msr_hv_runtime) {
             kvm_msr_entry_add(cpu, HV_X64_MSR_VP_RUNTIME, env->msr_hv_runtime);
         }
+        if (cpu->hyperv_vpindex && hv_vpindex_settable) {
+            kvm_msr_entry_add(cpu, HV_X64_MSR_VP_INDEX, hyperv_vp_index(cpu));
+        }
         if (cpu->hyperv_synic) {
             int j;
 
diff --git a/target/i386/kvm_i386.h b/target/i386/kvm_i386.h
index e5df24c..3057ba4 100644
--- a/target/i386/kvm_i386.h
+++ b/target/i386/kvm_i386.h
@@ -63,4 +63,6 @@ void kvm_put_apicbase(X86CPU *cpu, uint64_t value);
 
 bool kvm_enable_x2apic(void);
 bool kvm_has_x2apic_api(void);
+
+bool kvm_hv_vpindex_settable(void);
 #endif
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PULL 05/13] vhost-user-test: added proper TestServer *dest initialization in test_migrate()
  2018-07-17 15:06 [Qemu-devel] [PULL 00/13] Misc fixes for QEMU 3.0.0-rc1 Paolo Bonzini
                   ` (3 preceding siblings ...)
  2018-07-17 15:06 ` [Qemu-devel] [PULL 04/13] hyperv: ensure VP index equal to QEMU cpu_index Paolo Bonzini
@ 2018-07-17 15:06 ` Paolo Bonzini
  2018-07-17 15:06 ` [Qemu-devel] [PULL 06/13] PC Chipset: Improve serial divisor calculation Paolo Bonzini
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-17 15:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Emanuele Giuseppe Esposito

From: Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>

server->bus in _test_server_free() could be NULL, since TestServer
*dest in test_migrate() was not properly initialized like TestServer *s.
Added init_virtio_dev(dest) and uninit_virtio_dev(dest), so the fields
are properly set and when test_server_free(dest); is called, they can
be correctly freed.

The reason for that is init_virtio_dev() calls qpci_init_pc(), that
creates a QPCIBusPC * (returned as QPCIBus *), while test_server_free()
calls qpci_free_pc(), that frees the QPCIBus *. Not calling
init_virtio_dev() would leave the QPCIBus * of TestServer unset.

Problem came out once I modified  pci-pc.c and pci-pc.h, modifying
QPCIBusPC by adding another field before QPCIBus bus. Re-running the
tests showed vhost-user-test failing.

Signed-off-by: Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
Message-Id: <1530022733-29581-1-git-send-email-esposem@usi.ch>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/vhost-user-test.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 8ff2106..fecc832 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -537,6 +537,7 @@ static gboolean _test_server_free(TestServer *server)
     g_free(server->mig_path);
 
     g_free(server->chr_name);
+    g_assert(server->bus);
     qpci_free_pc(server->bus);
 
     g_free(server);
@@ -684,6 +685,7 @@ static void test_migrate(void)
     g_free(cmd);
 
     init_virtio_dev(s, 1u << VIRTIO_NET_F_MAC);
+    init_virtio_dev(dest, 1u << VIRTIO_NET_F_MAC);
     wait_for_fds(s);
     size = get_log_size(s);
     g_assert_cmpint(size, ==, (2 * 1024 * 1024) / (VHOST_LOG_PAGE * 8));
@@ -739,6 +741,7 @@ static void test_migrate(void)
     read_guest_mem_server(dest);
 
     uninit_virtio_dev(s);
+    uninit_virtio_dev(dest);
 
     g_source_destroy(source);
     g_source_unref(source);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PULL 06/13] PC Chipset: Improve serial divisor calculation
  2018-07-17 15:06 [Qemu-devel] [PULL 00/13] Misc fixes for QEMU 3.0.0-rc1 Paolo Bonzini
                   ` (4 preceding siblings ...)
  2018-07-17 15:06 ` [Qemu-devel] [PULL 05/13] vhost-user-test: added proper TestServer *dest initialization in test_migrate() Paolo Bonzini
@ 2018-07-17 15:06 ` Paolo Bonzini
  2018-07-17 15:06 ` [Qemu-devel] [PULL 07/13] hw/char/serial: retry write if EAGAIN Paolo Bonzini
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-17 15:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Calvin Lee

From: Calvin Lee <cyrus296@gmail.com>

This fixes several problems I found in the UART serial implementation.
Now all divisor values are allowed, while before divisor values of zero
and below the base baud rate were rejected. All changes are in reference
to http://www.sci.muni.cz/docs/pc/serport.txt

Signed-off-by: Calvin Lee <cyrus296@gmail.com>
Message-Id: <20180512000545.966-2-cyrus296@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/char/serial.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/hw/char/serial.c b/hw/char/serial.c
index cd7d747..d3b75f0 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -150,13 +150,10 @@ static void serial_update_irq(SerialState *s)
 
 static void serial_update_parameters(SerialState *s)
 {
-    int speed, parity, data_bits, stop_bits, frame_size;
+    float speed;
+    int parity, data_bits, stop_bits, frame_size;
     QEMUSerialSetParams ssp;
 
-    if (s->divider == 0 || s->divider > s->baudbase) {
-        return;
-    }
-
     /* Start bit. */
     frame_size = 1;
     if (s->lcr & 0x08) {
@@ -169,14 +166,16 @@ static void serial_update_parameters(SerialState *s)
     } else {
             parity = 'N';
     }
-    if (s->lcr & 0x04)
+    if (s->lcr & 0x04) {
         stop_bits = 2;
-    else
+    } else {
         stop_bits = 1;
+    }
 
     data_bits = (s->lcr & 0x03) + 5;
     frame_size += data_bits + stop_bits;
-    speed = s->baudbase / s->divider;
+    /* Zero divisor should give about 3500 baud */
+    speed = (s->divider == 0) ? 3500 : (float) s->baudbase / s->divider;
     ssp.speed = speed;
     ssp.parity = parity;
     ssp.data_bits = data_bits;
@@ -184,7 +183,7 @@ static void serial_update_parameters(SerialState *s)
     s->char_transmit_time =  (NANOSECONDS_PER_SECOND / speed) * frame_size;
     qemu_chr_fe_ioctl(&s->chr, CHR_IOCTL_SERIAL_SET_PARAMS, &ssp);
 
-    DPRINTF("speed=%d parity=%c data=%d stop=%d\n",
+    DPRINTF("speed=%.2f parity=%c data=%d stop=%d\n",
            speed, parity, data_bits, stop_bits);
 }
 
@@ -341,7 +340,11 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val,
     default:
     case 0:
         if (s->lcr & UART_LCR_DLAB) {
-            s->divider = (s->divider & 0xff00) | val;
+            if (size == 2) {
+                s->divider = (s->divider & 0xff00) | val;
+            } else if (size == 4) {
+                s->divider = val;
+            }
             serial_update_parameters(s);
         } else {
             s->thr = (uint8_t) val;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PULL 07/13] hw/char/serial: retry write if EAGAIN
  2018-07-17 15:06 [Qemu-devel] [PULL 00/13] Misc fixes for QEMU 3.0.0-rc1 Paolo Bonzini
                   ` (5 preceding siblings ...)
  2018-07-17 15:06 ` [Qemu-devel] [PULL 06/13] PC Chipset: Improve serial divisor calculation Paolo Bonzini
@ 2018-07-17 15:06 ` Paolo Bonzini
  2018-07-17 15:06 ` [Qemu-devel] [PULL 08/13] qdev: add HotplugHandler->post_plug() callback Paolo Bonzini
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-17 15:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

If the chardev returns -1 with EAGAIN errno on write(), it should try
to send it again (EINTR is handled by the chardev itself).

This fixes commit 019288bf137183bf3407c9824655b753bfafc99f
"hw/char/serial: Only retry if qemu_chr_fe_write returns 0"

Tested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20180716110755.12499-1-marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/char/serial.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/hw/char/serial.c b/hw/char/serial.c
index d3b75f0..251f40f 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -260,15 +260,20 @@ static void serial_xmit(SerialState *s)
         if (s->mcr & UART_MCR_LOOP) {
             /* in loopback mode, say that we just received a char */
             serial_receive1(s, &s->tsr, 1);
-        } else if (qemu_chr_fe_write(&s->chr, &s->tsr, 1) == 0 &&
-                   s->tsr_retry < MAX_XMIT_RETRY) {
-            assert(s->watch_tag == 0);
-            s->watch_tag =
-                qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
-                                      serial_watch_cb, s);
-            if (s->watch_tag > 0) {
-                s->tsr_retry++;
-                return;
+        } else {
+            int rc = qemu_chr_fe_write(&s->chr, &s->tsr, 1);
+
+            if ((rc == 0 ||
+                 (rc == -1 && errno == EAGAIN)) &&
+                s->tsr_retry < MAX_XMIT_RETRY) {
+                assert(s->watch_tag == 0);
+                s->watch_tag =
+                    qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
+                                          serial_watch_cb, s);
+                if (s->watch_tag > 0) {
+                    s->tsr_retry++;
+                    return;
+                }
             }
         }
         s->tsr_retry = 0;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PULL 08/13] qdev: add HotplugHandler->post_plug() callback
  2018-07-17 15:06 [Qemu-devel] [PULL 00/13] Misc fixes for QEMU 3.0.0-rc1 Paolo Bonzini
                   ` (6 preceding siblings ...)
  2018-07-17 15:06 ` [Qemu-devel] [PULL 07/13] hw/char/serial: retry write if EAGAIN Paolo Bonzini
@ 2018-07-17 15:06 ` Paolo Bonzini
  2018-07-17 15:06 ` [Qemu-devel] [PULL 09/13] virtio-scsi: fix hotplug ->reset() vs event race Paolo Bonzini
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-17 15:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi

From: Stefan Hajnoczi <stefanha@redhat.com>

The ->pre_plug() callback is invoked before the device is realized.  The
->plug() callback is invoked when the device is being realized but
before it is reset.

This patch adds a ->post_plug() callback which is invoked after the
device has been reset.  This callback is needed by HotplugHandlers that
need to wait until after ->reset().

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20180716083732.3347-2-stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/hotplug.c    | 10 ++++++++++
 hw/core/qdev.c       |  4 ++++
 include/hw/hotplug.h | 11 +++++++++++
 3 files changed, 25 insertions(+)

diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c
index 17ac986..2253072 100644
--- a/hw/core/hotplug.c
+++ b/hw/core/hotplug.c
@@ -35,6 +35,16 @@ void hotplug_handler_plug(HotplugHandler *plug_handler,
     }
 }
 
+void hotplug_handler_post_plug(HotplugHandler *plug_handler,
+                               DeviceState *plugged_dev)
+{
+    HotplugHandlerClass *hdc = HOTPLUG_HANDLER_GET_CLASS(plug_handler);
+
+    if (hdc->post_plug) {
+        hdc->post_plug(plug_handler, plugged_dev);
+    }
+}
+
 void hotplug_handler_unplug_request(HotplugHandler *plug_handler,
                                     DeviceState *plugged_dev,
                                     Error **errp)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index cf0db4b..529b82d 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -867,6 +867,10 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
             device_reset(dev);
         }
         dev->pending_deleted_event = false;
+
+        if (hotplug_ctrl) {
+            hotplug_handler_post_plug(hotplug_ctrl, dev);
+        }
     } else if (!value && dev->realized) {
         Error **local_errp = NULL;
         QLIST_FOREACH(bus, &dev->child_bus, sibling) {
diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h
index 1a0516a..51541d6 100644
--- a/include/hw/hotplug.h
+++ b/include/hw/hotplug.h
@@ -47,6 +47,8 @@ typedef void (*hotplug_fn)(HotplugHandler *plug_handler,
  * @parent: Opaque parent interface.
  * @pre_plug: pre plug callback called at start of device.realize(true)
  * @plug: plug callback called at end of device.realize(true).
+ * @post_plug: post plug callback called after device.realize(true) and device
+ *             reset
  * @unplug_request: unplug request callback.
  *                  Used as a means to initiate device unplug for devices that
  *                  require asynchronous unplug handling.
@@ -61,6 +63,7 @@ typedef struct HotplugHandlerClass {
     /* <public> */
     hotplug_fn pre_plug;
     hotplug_fn plug;
+    void (*post_plug)(HotplugHandler *plug_handler, DeviceState *plugged_dev);
     hotplug_fn unplug_request;
     hotplug_fn unplug;
 } HotplugHandlerClass;
@@ -84,6 +87,14 @@ void hotplug_handler_pre_plug(HotplugHandler *plug_handler,
                               Error **errp);
 
 /**
+ * hotplug_handler_post_plug:
+ *
+ * Call #HotplugHandlerClass.post_plug callback of @plug_handler.
+ */
+void hotplug_handler_post_plug(HotplugHandler *plug_handler,
+                               DeviceState *plugged_dev);
+
+/**
  * hotplug_handler_unplug_request:
  *
  * Calls #HotplugHandlerClass.unplug_request callback of @plug_handler.
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PULL 09/13] virtio-scsi: fix hotplug ->reset() vs event race
  2018-07-17 15:06 [Qemu-devel] [PULL 00/13] Misc fixes for QEMU 3.0.0-rc1 Paolo Bonzini
                   ` (7 preceding siblings ...)
  2018-07-17 15:06 ` [Qemu-devel] [PULL 08/13] qdev: add HotplugHandler->post_plug() callback Paolo Bonzini
@ 2018-07-17 15:06 ` Paolo Bonzini
  2018-07-17 15:06 ` [Qemu-devel] [PULL 10/13] i386: fix regression parsing multiboot initrd modules Paolo Bonzini
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-17 15:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stefan Hajnoczi, Fam Zheng

From: Stefan Hajnoczi <stefanha@redhat.com>

There is a race condition during hotplug when iothread is used.  It
occurs because virtio-scsi may be processing command queues in the
iothread while the monitor performs SCSI device hotplug.

When a SCSI device is hotplugged the HotplugHandler->plug() callback is
invoked and virtio-scsi emits a rescan event to the guest.

If the guest submits a SCSI command at this point then it may be
cancelled before hotplug completes.  This happens because ->reset() is
called by hw/core/qdev.c:device_set_realized() after
HotplugHandler->plug() has been called and
hw/scsi/scsi-disk.c:scsi_disk_reset() purges all requests.

This patch uses the new HotplugHandler->post_plug() callback to emit the
rescan event after ->reset().  This eliminates the race conditions where
requests could be cancelled.

Reported-by: l00284672 <lizhengui@huawei.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Fam Zheng <famz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20180716083732.3347-3-stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/virtio-scsi.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 3aa9971..5a3057d 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -797,8 +797,16 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev,
         virtio_scsi_acquire(s);
         blk_set_aio_context(sd->conf.blk, s->ctx);
         virtio_scsi_release(s);
-
     }
+}
+
+/* Announce the new device after it has been plugged */
+static void virtio_scsi_post_hotplug(HotplugHandler *hotplug_dev,
+                                     DeviceState *dev)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(hotplug_dev);
+    VirtIOSCSI *s = VIRTIO_SCSI(vdev);
+    SCSIDevice *sd = SCSI_DEVICE(dev);
 
     if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) {
         virtio_scsi_acquire(s);
@@ -968,6 +976,7 @@ static void virtio_scsi_class_init(ObjectClass *klass, void *data)
     vdc->start_ioeventfd = virtio_scsi_dataplane_start;
     vdc->stop_ioeventfd = virtio_scsi_dataplane_stop;
     hc->plug = virtio_scsi_hotplug;
+    hc->post_plug = virtio_scsi_post_hotplug;
     hc->unplug = virtio_scsi_hotunplug;
 }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PULL 10/13] i386: fix regression parsing multiboot initrd modules
  2018-07-17 15:06 [Qemu-devel] [PULL 00/13] Misc fixes for QEMU 3.0.0-rc1 Paolo Bonzini
                   ` (8 preceding siblings ...)
  2018-07-17 15:06 ` [Qemu-devel] [PULL 09/13] virtio-scsi: fix hotplug ->reset() vs event race Paolo Bonzini
@ 2018-07-17 15:06 ` Paolo Bonzini
  2018-07-17 15:06 ` [Qemu-devel] [PULL 11/13] i386: only parse the initrd_filename once for multiboot modules Paolo Bonzini
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-17 15:06 UTC (permalink / raw)
  To: qemu-devel

From: Daniel P. Berrangé <berrange@redhat.com>

The logic for parsing the multiboot initrd modules was messed up in

  commit 950c4e6c94b15cd0d8b63891dddd7a8dbf458e6a
  Author: Daniel P. Berrangé <berrange@redhat.com>
  Date:   Mon Apr 16 12:17:43 2018 +0100

    opts: don't silently truncate long option values

Causing the length to be undercounter, and the number of modules over
counted. It also passes NULL to get_opt_value() which was not robust
at accepting a NULL value.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20180514171913.17664-2-berrange@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Tested-by: Roman Kagan <rkagan@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/multiboot.c | 3 +--
 util/qemu-option.c  | 4 +++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index 7a2953e..8e26545 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -292,8 +292,7 @@ int load_multiboot(FWCfgState *fw_cfg,
     cmdline_len += strlen(kernel_cmdline) + 1;
     if (initrd_filename) {
         const char *r = get_opt_value(initrd_filename, NULL);
-        cmdline_len += strlen(r) + 1;
-        mbs.mb_mods_avail = 1;
+        cmdline_len += strlen(initrd_filename) + 1;
         while (1) {
             mbs.mb_mods_avail++;
             r = get_opt_value(r, NULL);
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 19761e3..834217f 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -75,7 +75,9 @@ const char *get_opt_value(const char *p, char **value)
     size_t capacity = 0, length;
     const char *offset;
 
-    *value = NULL;
+    if (value) {
+        *value = NULL;
+    }
     while (1) {
         offset = qemu_strchrnul(p, ',');
         length = offset - p;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PULL 11/13] i386: only parse the initrd_filename once for multiboot modules
  2018-07-17 15:06 [Qemu-devel] [PULL 00/13] Misc fixes for QEMU 3.0.0-rc1 Paolo Bonzini
                   ` (9 preceding siblings ...)
  2018-07-17 15:06 ` [Qemu-devel] [PULL 10/13] i386: fix regression parsing multiboot initrd modules Paolo Bonzini
@ 2018-07-17 15:06 ` Paolo Bonzini
  2018-07-17 15:06 ` [Qemu-devel] [PULL 12/13] opts: remove redundant check for NULL parameter Paolo Bonzini
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-17 15:06 UTC (permalink / raw)
  To: qemu-devel

From: Daniel P. Berrangé <berrange@redhat.com>

The multiboot code parses the initrd_filename twice, first to count how
many entries there are, and second to process each entry. This changes
the first loop to store the parse module names in a list, and the second
loop can now use these names. This avoids having to pass NULL to the
get_opt_value() method which means it can safely assume a non-NULL param.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20180514171913.17664-3-berrange@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Tested-by: Roman Kagan <rkagan@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/multiboot.c | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index 8e26545..d519e20 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -161,6 +161,7 @@ int load_multiboot(FWCfgState *fw_cfg,
     uint8_t bootinfo[MBI_SIZE];
     uint8_t *mb_bootinfo_data;
     uint32_t cmdline_len;
+    GList *mods = NULL;
 
     /* Ok, let's see if it is a multiboot image.
        The header is 12x32bit long, so the latest entry may be 8192 - 48. */
@@ -291,15 +292,16 @@ int load_multiboot(FWCfgState *fw_cfg,
     cmdline_len = strlen(kernel_filename) + 1;
     cmdline_len += strlen(kernel_cmdline) + 1;
     if (initrd_filename) {
-        const char *r = get_opt_value(initrd_filename, NULL);
+        const char *r = initrd_filename;
         cmdline_len += strlen(initrd_filename) + 1;
-        while (1) {
+        while (*r) {
+            char *value;
+            r = get_opt_value(r, &value);
             mbs.mb_mods_avail++;
-            r = get_opt_value(r, NULL);
-            if (!*r) {
-                break;
+            mods = g_list_append(mods, value);
+            if (*r) {
+                r++;
             }
-            r++;
         }
     }
 
@@ -314,20 +316,16 @@ int load_multiboot(FWCfgState *fw_cfg,
     mbs.offset_cmdlines   = mbs.offset_mbinfo + mbs.mb_mods_avail * MB_MOD_SIZE;
     mbs.offset_bootloader = mbs.offset_cmdlines + cmdline_len;
 
-    if (initrd_filename) {
-        const char *next_initrd;
-        char not_last;
-        char *one_file = NULL;
-
+    if (mods) {
+        GList *tmpl = mods;
         mbs.offset_mods = mbs.mb_buf_size;
 
-        do {
+        while (tmpl) {
             char *next_space;
             int mb_mod_length;
             uint32_t offs = mbs.mb_buf_size;
+            char *one_file = tmpl->data;
 
-            next_initrd = get_opt_value(initrd_filename, &one_file);
-            not_last = *next_initrd;
             /* if a space comes after the module filename, treat everything
                after that as parameters */
             hwaddr c = mb_add_cmdline(&mbs, one_file);
@@ -352,10 +350,10 @@ int load_multiboot(FWCfgState *fw_cfg,
             mb_debug("mod_start: %p\nmod_end:   %p\n  cmdline: "TARGET_FMT_plx,
                      (char *)mbs.mb_buf + offs,
                      (char *)mbs.mb_buf + offs + mb_mod_length, c);
-            initrd_filename = next_initrd+1;
             g_free(one_file);
-            one_file = NULL;
-        } while (not_last);
+            tmpl = tmpl->next;
+        }
+        g_list_free(mods);
     }
 
     /* Commandline support */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PULL 12/13] opts: remove redundant check for NULL parameter
  2018-07-17 15:06 [Qemu-devel] [PULL 00/13] Misc fixes for QEMU 3.0.0-rc1 Paolo Bonzini
                   ` (10 preceding siblings ...)
  2018-07-17 15:06 ` [Qemu-devel] [PULL 11/13] i386: only parse the initrd_filename once for multiboot modules Paolo Bonzini
@ 2018-07-17 15:06 ` Paolo Bonzini
  2018-07-17 15:06 ` [Qemu-devel] [PULL 13/13] Document command line options with single dash Paolo Bonzini
  2018-07-17 15:16 ` [Qemu-devel] [PULL 00/13] Misc fixes for QEMU 3.0.0-rc1 Peter Maydell
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-17 15:06 UTC (permalink / raw)
  To: qemu-devel

From: Daniel P. Berrangé <berrange@redhat.com>

No callers of get_opt_value() pass in a NULL for the "value" parameter,
so the check is redundant.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20180514171913.17664-4-berrange@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Tested-by: Roman Kagan <rkagan@virtuozzo.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 util/qemu-option.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/util/qemu-option.c b/util/qemu-option.c
index 834217f..01886ef 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -75,20 +75,16 @@ const char *get_opt_value(const char *p, char **value)
     size_t capacity = 0, length;
     const char *offset;
 
-    if (value) {
-        *value = NULL;
-    }
+    *value = NULL;
     while (1) {
         offset = qemu_strchrnul(p, ',');
         length = offset - p;
         if (*offset != '\0' && *(offset + 1) == ',') {
             length++;
         }
-        if (value) {
-            *value = g_renew(char, *value, capacity + length + 1);
-            strncpy(*value + capacity, p, length);
-            (*value)[capacity + length] = '\0';
-        }
+        *value = g_renew(char, *value, capacity + length + 1);
+        strncpy(*value + capacity, p, length);
+        (*value)[capacity + length] = '\0';
         capacity += length;
         if (*offset == '\0' ||
             *(offset + 1) != ',') {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Qemu-devel] [PULL 13/13] Document command line options with single dash
  2018-07-17 15:06 [Qemu-devel] [PULL 00/13] Misc fixes for QEMU 3.0.0-rc1 Paolo Bonzini
                   ` (11 preceding siblings ...)
  2018-07-17 15:06 ` [Qemu-devel] [PULL 12/13] opts: remove redundant check for NULL parameter Paolo Bonzini
@ 2018-07-17 15:06 ` Paolo Bonzini
  2018-07-17 15:16 ` [Qemu-devel] [PULL 00/13] Misc fixes for QEMU 3.0.0-rc1 Peter Maydell
  13 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2018-07-17 15:06 UTC (permalink / raw)
  To: qemu-devel

From: BALATON Zoltan <balaton@eik.bme.hu>

QEMU options have a single dash (but also work as double dash for
convenience and compatibility). Most options are listed with single
dash in command line help but some were listed with two dashes.
Normalize these to have the same format as the others.

Left --preconfig as that is mentioned as double dash everywhere so I
assume that is the preferred form for that.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Acked-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20180716193312.A5BA17456B9@zero.eik.bme.hu>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-options.hx | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 654e69c..1a186fb 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -471,7 +471,7 @@ STEXI
 @item -balloon virtio[,addr=@var{addr}]
 @findex -balloon
 Enable virtio balloon device, optionally with PCI address @var{addr}. This
-option is deprecated, use @option{--device virtio-balloon} instead.
+option is deprecated, use @option{-device virtio-balloon} instead.
 ETEXI
 
 DEF("device", HAS_ARG, QEMU_OPTION_device,
@@ -2005,7 +2005,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
     "-netdev hubport,id=str,hubid=n[,netdev=nd]\n"
     "                configure a hub port on the hub with ID 'n'\n", QEMU_ARCH_ALL)
 DEF("nic", HAS_ARG, QEMU_OPTION_nic,
-    "--nic [tap|bridge|"
+    "-nic [tap|bridge|"
 #ifdef CONFIG_SLIRP
     "user|"
 #endif
@@ -2024,7 +2024,7 @@ DEF("nic", HAS_ARG, QEMU_OPTION_nic,
     "socket][,option][,...][mac=macaddr]\n"
     "                initialize an on-board / default host NIC (using MAC address\n"
     "                macaddr) and connect it to the given host network backend\n"
-    "--nic none      use it alone to have zero network devices (the default is to\n"
+    "-nic none       use it alone to have zero network devices (the default is to\n"
     "                provided a 'user' network connection)\n",
     QEMU_ARCH_ALL)
 DEF("net", HAS_ARG, QEMU_OPTION_net,
@@ -3338,7 +3338,7 @@ mlocking qemu and guest memory can be enabled via @option{mlock=on}
 ETEXI
 
 DEF("overcommit", HAS_ARG, QEMU_OPTION_overcommit,
-    "--overcommit [mem-lock=on|off][cpu-pm=on|off]\n"
+    "-overcommit [mem-lock=on|off][cpu-pm=on|off]\n"
     "                run qemu with overcommit hints\n"
     "                mem-lock=on|off controls memory lock support (default: off)\n"
     "                cpu-pm=on|off controls cpu power management (default: off)\n",
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [Qemu-devel] [PULL 00/13] Misc fixes for QEMU 3.0.0-rc1
  2018-07-17 15:06 [Qemu-devel] [PULL 00/13] Misc fixes for QEMU 3.0.0-rc1 Paolo Bonzini
                   ` (12 preceding siblings ...)
  2018-07-17 15:06 ` [Qemu-devel] [PULL 13/13] Document command line options with single dash Paolo Bonzini
@ 2018-07-17 15:16 ` Peter Maydell
  13 siblings, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2018-07-17 15:16 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: QEMU Developers

On 17 July 2018 at 16:06, Paolo Bonzini <pbonzini@redhat.com> wrote:
> The following changes since commit 9277d81f5c2c6f4d0b5e47c8476eb7ee7e5c0beb:
>
>   docs: Grammar and spelling fixes (2018-07-13 10:16:04 +0100)
>
> are available in the git repository at:
>
>   git://github.com/bonzini/qemu.git

Hi -- your pullreq seems to be missing the tag name to pull...

thanks
-- PMM

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2018-07-17 15:17 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-17 15:06 [Qemu-devel] [PULL 00/13] Misc fixes for QEMU 3.0.0-rc1 Paolo Bonzini
2018-07-17 15:06 ` [Qemu-devel] [PULL 01/13] dump: add kernel_gs_base to QEMU CPU state Paolo Bonzini
2018-07-17 15:06 ` [Qemu-devel] [PULL 02/13] accel: Fix typo and grammar in comment Paolo Bonzini
2018-07-17 15:06 ` [Qemu-devel] [PULL 03/13] hyperv: rename vcpu_id to vp_index Paolo Bonzini
2018-07-17 15:06 ` [Qemu-devel] [PULL 04/13] hyperv: ensure VP index equal to QEMU cpu_index Paolo Bonzini
2018-07-17 15:06 ` [Qemu-devel] [PULL 05/13] vhost-user-test: added proper TestServer *dest initialization in test_migrate() Paolo Bonzini
2018-07-17 15:06 ` [Qemu-devel] [PULL 06/13] PC Chipset: Improve serial divisor calculation Paolo Bonzini
2018-07-17 15:06 ` [Qemu-devel] [PULL 07/13] hw/char/serial: retry write if EAGAIN Paolo Bonzini
2018-07-17 15:06 ` [Qemu-devel] [PULL 08/13] qdev: add HotplugHandler->post_plug() callback Paolo Bonzini
2018-07-17 15:06 ` [Qemu-devel] [PULL 09/13] virtio-scsi: fix hotplug ->reset() vs event race Paolo Bonzini
2018-07-17 15:06 ` [Qemu-devel] [PULL 10/13] i386: fix regression parsing multiboot initrd modules Paolo Bonzini
2018-07-17 15:06 ` [Qemu-devel] [PULL 11/13] i386: only parse the initrd_filename once for multiboot modules Paolo Bonzini
2018-07-17 15:06 ` [Qemu-devel] [PULL 12/13] opts: remove redundant check for NULL parameter Paolo Bonzini
2018-07-17 15:06 ` [Qemu-devel] [PULL 13/13] Document command line options with single dash Paolo Bonzini
2018-07-17 15:16 ` [Qemu-devel] [PULL 00/13] Misc fixes for QEMU 3.0.0-rc1 Peter Maydell

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).