All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 0/7] serial device hotplug patch series.
@ 2012-10-12  9:25 Gerd Hoffmann
  2012-10-12  9:25 ` [Qemu-devel] [PATCH 1/7] serial: split serial.c Gerd Hoffmann
                   ` (7 more replies)
  0 siblings, 8 replies; 24+ messages in thread
From: Gerd Hoffmann @ 2012-10-12  9:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

  Hi,

This patch series tackles serial device hotplug.

The first four patches have been on the list before, they implement
pci-serial devices featuring a hot-pluggable 16550 uart and got some
minor tweaks only.

The next two patches update the usb-serial device.  It will only show up
in the guest when the chardev is open.  You'll see the difference with
socket chardevs:  If you open the chardev (by connecting to the socket)
the device will show up in the guest, on close (disconnect) it will
disappear.

Final patch adds chardev hotplug to the mix, which makes the other
patches alot more useful.  It is the missing bit needed to really
hotplug serial devices:

   (qemu) chardev_add pciserial file /root/hotchardev.log
   (qemu) device_add pci-serial,chardev=pciserial,id=pciserial

And the reverse:

   (qemu) device_del pciserial
   (qemu) chardev_del pciserial

please review & pull,
  Gerd

The following changes since commit b4ae3cfa57b8c1bdbbd7b7d420971e9171203ade:

  ssi: Add slave autoconnect helper (2012-10-10 11:13:32 +1000)

are available in the git repository at:
  git://git.kraxel.org/qemu serial.1

Gerd Hoffmann (7):
      serial: split serial.c
      serial: add pci variant
      serial: add windows inf file for the pci card to docs
      serial: add 2x + 4x pci variant
      usb-serial: don't magically zap chardev on umplug
      usb-serial: only expose device in guest when the chardev is open
      chardev: add hotplug support.

 default-configs/pci.mak  |    2 +
 docs/qemupciserial.inf   |  109 ++++++++++++++++++
 hmp-commands.hx          |   49 ++++++++
 hmp.c                    |   39 +++++++
 hmp.h                    |    2 +
 hw/Makefile.objs         |    3 +-
 hw/alpha_dp264.c         |    1 +
 hw/kzm.c                 |    2 +-
 hw/mips_fulong2e.c       |    1 +
 hw/mips_jazz.c           |    1 +
 hw/mips_malta.c          |    1 +
 hw/mips_mipssim.c        |    2 +-
 hw/mips_r4k.c            |    1 +
 hw/musicpal.c            |    2 +-
 hw/omap_uart.c           |    3 +-
 hw/openrisc_sim.c        |    3 +-
 hw/pc.c                  |    1 +
 hw/pc.h                  |   27 -----
 hw/pci_ids.h             |    1 +
 hw/petalogix_ml605_mmu.c |    2 +-
 hw/ppc/e500.c            |    2 +-
 hw/ppc405_uc.c           |    2 +-
 hw/ppc440_bamboo.c       |    2 +-
 hw/ppc_prep.c            |    1 +
 hw/pxa2xx.c              |    2 +-
 hw/serial-isa.c          |  130 ++++++++++++++++++++++
 hw/serial-pci.c          |  272 ++++++++++++++++++++++++++++++++++++++++++++++
 hw/serial.c              |  149 ++-----------------------
 hw/serial.h              |   74 +++++++++++++
 hw/sm501.c               |    2 +-
 hw/sun4u.c               |    1 +
 hw/usb/dev-serial.c      |   21 +++-
 hw/virtex_ml507.c        |    2 +-
 hw/xtensa_lx60.c         |    3 +-
 qapi-schema.json         |   46 ++++++++
 qemu-char.c              |   74 +++++++++++++
 qmp-commands.hx          |   61 ++++++++++
 37 files changed, 913 insertions(+), 183 deletions(-)
 create mode 100644 docs/qemupciserial.inf
 create mode 100644 hw/serial-isa.c
 create mode 100644 hw/serial-pci.c
 create mode 100644 hw/serial.h

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

* [Qemu-devel] [PATCH 1/7] serial: split serial.c
  2012-10-12  9:25 [Qemu-devel] [PULL 0/7] serial device hotplug patch series Gerd Hoffmann
@ 2012-10-12  9:25 ` Gerd Hoffmann
  2012-10-12  9:25 ` [Qemu-devel] [PATCH 2/7] serial: add pci variant Gerd Hoffmann
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Gerd Hoffmann @ 2012-10-12  9:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Split serial.c into serial.c, serial.h and serial-isa.c.  While being at
creating a serial.h header file move the serial prototypes from pc.h to
the new serial.h.  The latter leads to s/pc.h/serial.h/ in tons of
boards which just want the serial bits from pc.h

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/Makefile.objs         |    2 +-
 hw/alpha_dp264.c         |    1 +
 hw/kzm.c                 |    2 +-
 hw/mips_fulong2e.c       |    1 +
 hw/mips_jazz.c           |    1 +
 hw/mips_malta.c          |    1 +
 hw/mips_mipssim.c        |    2 +-
 hw/mips_r4k.c            |    1 +
 hw/musicpal.c            |    2 +-
 hw/omap_uart.c           |    3 +-
 hw/openrisc_sim.c        |    3 +-
 hw/pc.c                  |    1 +
 hw/pc.h                  |   27 ---------
 hw/petalogix_ml605_mmu.c |    2 +-
 hw/ppc/e500.c            |    2 +-
 hw/ppc405_uc.c           |    2 +-
 hw/ppc440_bamboo.c       |    2 +-
 hw/ppc_prep.c            |    1 +
 hw/pxa2xx.c              |    2 +-
 hw/serial-isa.c          |  130 +++++++++++++++++++++++++++++++++++++++++
 hw/serial.c              |  143 ++--------------------------------------------
 hw/serial.h              |   73 +++++++++++++++++++++++
 hw/sm501.c               |    2 +-
 hw/sun4u.c               |    1 +
 hw/virtex_ml507.c        |    2 +-
 hw/xtensa_lx60.c         |    3 +-
 26 files changed, 232 insertions(+), 180 deletions(-)
 create mode 100644 hw/serial-isa.c
 create mode 100644 hw/serial.h

diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 854faa9..16e7a1e 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -20,7 +20,7 @@ common-obj-$(CONFIG_M48T59) += m48t59.o
 common-obj-$(CONFIG_ESCC) += escc.o
 common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
 
-common-obj-$(CONFIG_SERIAL) += serial.o
+common-obj-$(CONFIG_SERIAL) += serial.o serial-isa.o
 common-obj-$(CONFIG_PARALLEL) += parallel.o
 common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
 common-obj-$(CONFIG_PCSPK) += pcspk.o
diff --git a/hw/alpha_dp264.c b/hw/alpha_dp264.c
index 5ea04c7..8ce04e5 100644
--- a/hw/alpha_dp264.c
+++ b/hw/alpha_dp264.c
@@ -15,6 +15,7 @@
 #include "mc146818rtc.h"
 #include "ide.h"
 #include "i8254.h"
+#include "serial.h"
 
 #define MAX_IDE_BUS 2
 
diff --git a/hw/kzm.c b/hw/kzm.c
index 68cd1b4..1f3082b 100644
--- a/hw/kzm.c
+++ b/hw/kzm.c
@@ -21,7 +21,7 @@
 #include "net.h"
 #include "sysemu.h"
 #include "boards.h"
-#include "pc.h" /* for the FPGA UART that emulates a 16550 */
+#include "serial.h"
 #include "imx.h"
 
     /* Memory map for Kzm Emulation Baseboard:
diff --git a/hw/mips_fulong2e.c b/hw/mips_fulong2e.c
index d4a8672..a3cb3ab 100644
--- a/hw/mips_fulong2e.c
+++ b/hw/mips_fulong2e.c
@@ -20,6 +20,7 @@
 
 #include "hw.h"
 #include "pc.h"
+#include "serial.h"
 #include "fdc.h"
 #include "net.h"
 #include "boards.h"
diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c
index db927f1..d35cd54 100644
--- a/hw/mips_jazz.c
+++ b/hw/mips_jazz.c
@@ -26,6 +26,7 @@
 #include "mips.h"
 #include "mips_cpudevs.h"
 #include "pc.h"
+#include "serial.h"
 #include "isa.h"
 #include "fdc.h"
 #include "sysemu.h"
diff --git a/hw/mips_malta.c b/hw/mips_malta.c
index 632b466..8f73b1b 100644
--- a/hw/mips_malta.c
+++ b/hw/mips_malta.c
@@ -24,6 +24,7 @@
 
 #include "hw.h"
 #include "pc.h"
+#include "serial.h"
 #include "fdc.h"
 #include "net.h"
 #include "boards.h"
diff --git a/hw/mips_mipssim.c b/hw/mips_mipssim.c
index 830f635..0ee6756 100644
--- a/hw/mips_mipssim.c
+++ b/hw/mips_mipssim.c
@@ -27,7 +27,7 @@
 #include "hw.h"
 #include "mips.h"
 #include "mips_cpudevs.h"
-#include "pc.h"
+#include "serial.h"
 #include "isa.h"
 #include "net.h"
 #include "sysemu.h"
diff --git a/hw/mips_r4k.c b/hw/mips_r4k.c
index 967a76e..b3be80b 100644
--- a/hw/mips_r4k.c
+++ b/hw/mips_r4k.c
@@ -11,6 +11,7 @@
 #include "mips.h"
 #include "mips_cpudevs.h"
 #include "pc.h"
+#include "serial.h"
 #include "isa.h"
 #include "net.h"
 #include "sysemu.h"
diff --git a/hw/musicpal.c b/hw/musicpal.c
index f305e21..346fe41 100644
--- a/hw/musicpal.c
+++ b/hw/musicpal.c
@@ -15,7 +15,7 @@
 #include "net.h"
 #include "sysemu.h"
 #include "boards.h"
-#include "pc.h"
+#include "serial.h"
 #include "qemu-timer.h"
 #include "ptimer.h"
 #include "block.h"
diff --git a/hw/omap_uart.c b/hw/omap_uart.c
index 167d5c4..1c16a54 100644
--- a/hw/omap_uart.c
+++ b/hw/omap_uart.c
@@ -20,8 +20,7 @@
 #include "qemu-char.h"
 #include "hw.h"
 #include "omap.h"
-/* We use pc-style serial ports.  */
-#include "pc.h"
+#include "serial.h"
 #include "exec-memory.h"
 
 /* UARTs */
diff --git a/hw/openrisc_sim.c b/hw/openrisc_sim.c
index 55e97f0..e484613 100644
--- a/hw/openrisc_sim.c
+++ b/hw/openrisc_sim.c
@@ -21,7 +21,8 @@
 #include "hw.h"
 #include "boards.h"
 #include "elf.h"
-#include "pc.h"
+#include "serial.h"
+#include "net.h"
 #include "loader.h"
 #include "exec-memory.h"
 #include "sysemu.h"
diff --git a/hw/pc.c b/hw/pc.c
index 6c0722d..805e8ca 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -23,6 +23,7 @@
  */
 #include "hw.h"
 #include "pc.h"
+#include "serial.h"
 #include "apic.h"
 #include "fdc.h"
 #include "ide.h"
diff --git a/hw/pc.h b/hw/pc.h
index 9923d96..6cba7ce 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -12,33 +12,6 @@
 
 /* PC-style peripherals (also used by other machines).  */
 
-/* serial.c */
-
-SerialState *serial_init(int base, qemu_irq irq, int baudbase,
-                         CharDriverState *chr);
-SerialState *serial_mm_init(MemoryRegion *address_space,
-                            target_phys_addr_t base, int it_shift,
-                            qemu_irq irq, int baudbase,
-                            CharDriverState *chr, enum device_endian);
-static inline bool serial_isa_init(ISABus *bus, int index,
-                                   CharDriverState *chr)
-{
-    ISADevice *dev;
-
-    dev = isa_try_create(bus, "isa-serial");
-    if (!dev) {
-        return false;
-    }
-    qdev_prop_set_uint32(&dev->qdev, "index", index);
-    qdev_prop_set_chr(&dev->qdev, "chardev", chr);
-    if (qdev_init(&dev->qdev) < 0) {
-        return false;
-    }
-    return true;
-}
-
-void serial_set_frequency(SerialState *s, uint32_t frequency);
-
 /* parallel.c */
 static inline bool parallel_init(ISABus *bus, int index, CharDriverState *chr)
 {
diff --git a/hw/petalogix_ml605_mmu.c b/hw/petalogix_ml605_mmu.c
index b9bfbed..1c12c5b 100644
--- a/hw/petalogix_ml605_mmu.c
+++ b/hw/petalogix_ml605_mmu.c
@@ -34,7 +34,7 @@
 #include "boards.h"
 #include "xilinx.h"
 #include "blockdev.h"
-#include "pc.h"
+#include "serial.h"
 #include "exec-memory.h"
 #include "ssi.h"
 
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index d23f9b2..846f53a 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -19,7 +19,7 @@
 #include "e500.h"
 #include "net.h"
 #include "hw/hw.h"
-#include "hw/pc.h"
+#include "hw/serial.h"
 #include "hw/pci.h"
 #include "hw/boards.h"
 #include "sysemu.h"
diff --git a/hw/ppc405_uc.c b/hw/ppc405_uc.c
index b52ab2f..e81409d 100644
--- a/hw/ppc405_uc.c
+++ b/hw/ppc405_uc.c
@@ -24,7 +24,7 @@
 #include "hw.h"
 #include "ppc.h"
 #include "ppc405.h"
-#include "pc.h"
+#include "serial.h"
 #include "qemu-timer.h"
 #include "sysemu.h"
 #include "qemu-log.h"
diff --git a/hw/ppc440_bamboo.c b/hw/ppc440_bamboo.c
index c198071..7e6fa85 100644
--- a/hw/ppc440_bamboo.c
+++ b/hw/ppc440_bamboo.c
@@ -23,7 +23,7 @@
 #include "loader.h"
 #include "elf.h"
 #include "exec-memory.h"
-#include "pc.h"
+#include "serial.h"
 #include "ppc.h"
 #include "ppc405.h"
 #include "sysemu.h"
diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index 1544430..50fd567 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -24,6 +24,7 @@
 #include "hw.h"
 #include "nvram.h"
 #include "pc.h"
+#include "serial.h"
 #include "fdc.h"
 #include "net.h"
 #include "sysemu.h"
diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c
index d5f1420..4ec904f 100644
--- a/hw/pxa2xx.c
+++ b/hw/pxa2xx.c
@@ -10,7 +10,7 @@
 #include "sysbus.h"
 #include "pxa.h"
 #include "sysemu.h"
-#include "pc.h"
+#include "serial.h"
 #include "i2c.h"
 #include "ssi.h"
 #include "qemu-char.h"
diff --git a/hw/serial-isa.c b/hw/serial-isa.c
new file mode 100644
index 0000000..96c78f7
--- /dev/null
+++ b/hw/serial-isa.c
@@ -0,0 +1,130 @@
+/*
+ * QEMU 16550A UART emulation
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ * Copyright (c) 2008 Citrix Systems, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "serial.h"
+#include "isa.h"
+
+typedef struct ISASerialState {
+    ISADevice dev;
+    uint32_t index;
+    uint32_t iobase;
+    uint32_t isairq;
+    SerialState state;
+} ISASerialState;
+
+static const int isa_serial_io[MAX_SERIAL_PORTS] = {
+    0x3f8, 0x2f8, 0x3e8, 0x2e8
+};
+static const int isa_serial_irq[MAX_SERIAL_PORTS] = {
+    4, 3, 4, 3
+};
+
+static int serial_isa_initfn(ISADevice *dev)
+{
+    static int index;
+    ISASerialState *isa = DO_UPCAST(ISASerialState, dev, dev);
+    SerialState *s = &isa->state;
+
+    if (isa->index == -1) {
+        isa->index = index;
+    }
+    if (isa->index >= MAX_SERIAL_PORTS) {
+        return -1;
+    }
+    if (isa->iobase == -1) {
+        isa->iobase = isa_serial_io[isa->index];
+    }
+    if (isa->isairq == -1) {
+        isa->isairq = isa_serial_irq[isa->index];
+    }
+    index++;
+
+    s->baudbase = 115200;
+    isa_init_irq(dev, &s->irq, isa->isairq);
+    serial_init_core(s);
+    qdev_set_legacy_instance_id(&dev->qdev, isa->iobase, 3);
+
+    memory_region_init_io(&s->io, &serial_io_ops, s, "serial", 8);
+    isa_register_ioport(dev, &s->io, isa->iobase);
+    return 0;
+}
+
+static const VMStateDescription vmstate_isa_serial = {
+    .name = "serial",
+    .version_id = 3,
+    .minimum_version_id = 2,
+    .fields = (VMStateField[]) {
+        VMSTATE_STRUCT(state, ISASerialState, 0, vmstate_serial, SerialState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static Property serial_isa_properties[] = {
+    DEFINE_PROP_UINT32("index",  ISASerialState, index,   -1),
+    DEFINE_PROP_HEX32("iobase",  ISASerialState, iobase,  -1),
+    DEFINE_PROP_UINT32("irq",    ISASerialState, isairq,  -1),
+    DEFINE_PROP_CHR("chardev",   ISASerialState, state.chr),
+    DEFINE_PROP_UINT32("wakeup", ISASerialState, state.wakeup, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void serial_isa_class_initfn(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
+    ic->init = serial_isa_initfn;
+    dc->vmsd = &vmstate_isa_serial;
+    dc->props = serial_isa_properties;
+}
+
+static TypeInfo serial_isa_info = {
+    .name          = "isa-serial",
+    .parent        = TYPE_ISA_DEVICE,
+    .instance_size = sizeof(ISASerialState),
+    .class_init    = serial_isa_class_initfn,
+};
+
+static void serial_register_types(void)
+{
+    type_register_static(&serial_isa_info);
+}
+
+type_init(serial_register_types)
+
+bool serial_isa_init(ISABus *bus, int index, CharDriverState *chr)
+{
+    ISADevice *dev;
+
+    dev = isa_try_create(bus, "isa-serial");
+    if (!dev) {
+        return false;
+    }
+    qdev_prop_set_uint32(&dev->qdev, "index", index);
+    qdev_prop_set_chr(&dev->qdev, "chardev", chr);
+    if (qdev_init(&dev->qdev) < 0) {
+        return false;
+    }
+    return true;
+}
diff --git a/hw/serial.c b/hw/serial.c
index a421d1e..78e219d 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -22,12 +22,10 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
-#include "hw.h"
+
+#include "serial.h"
 #include "qemu-char.h"
-#include "isa.h"
-#include "pc.h"
 #include "qemu-timer.h"
-#include "sysemu.h"
 
 //#define DEBUG_SERIAL
 
@@ -93,8 +91,6 @@
 #define UART_FCR_RFR        0x02    /* RCVR Fifo Reset */
 #define UART_FCR_FE         0x01    /* FIFO Enable */
 
-#define UART_FIFO_LENGTH    16      /* 16550A Fifo Length */
-
 #define XMIT_FIFO           0
 #define RECV_FIFO           1
 #define MAX_XMIT_RETRY      4
@@ -107,64 +103,6 @@ do { fprintf(stderr, "serial: " fmt , ## __VA_ARGS__); } while (0)
 do {} while (0)
 #endif
 
-typedef struct SerialFIFO {
-    uint8_t data[UART_FIFO_LENGTH];
-    uint8_t count;
-    uint8_t itl;                        /* Interrupt Trigger Level */
-    uint8_t tail;
-    uint8_t head;
-} SerialFIFO;
-
-struct SerialState {
-    uint16_t divider;
-    uint8_t rbr; /* receive register */
-    uint8_t thr; /* transmit holding register */
-    uint8_t tsr; /* transmit shift register */
-    uint8_t ier;
-    uint8_t iir; /* read only */
-    uint8_t lcr;
-    uint8_t mcr;
-    uint8_t lsr; /* read only */
-    uint8_t msr; /* read only */
-    uint8_t scr;
-    uint8_t fcr;
-    uint8_t fcr_vmstate; /* we can't write directly this value
-                            it has side effects */
-    /* NOTE: this hidden state is necessary for tx irq generation as
-       it can be reset while reading iir */
-    int thr_ipending;
-    qemu_irq irq;
-    CharDriverState *chr;
-    int last_break_enable;
-    int it_shift;
-    int baudbase;
-    int tsr_retry;
-    uint32_t wakeup;
-
-    uint64_t last_xmit_ts;              /* Time when the last byte was successfully sent out of the tsr */
-    SerialFIFO recv_fifo;
-    SerialFIFO xmit_fifo;
-
-    struct QEMUTimer *fifo_timeout_timer;
-    int timeout_ipending;                   /* timeout interrupt pending state */
-    struct QEMUTimer *transmit_timer;
-
-
-    uint64_t char_transmit_time;               /* time to transmit a char in ticks*/
-    int poll_msl;
-
-    struct QEMUTimer *modem_status_poll;
-    MemoryRegion io;
-};
-
-typedef struct ISASerialState {
-    ISADevice dev;
-    uint32_t index;
-    uint32_t iobase;
-    uint32_t isairq;
-    SerialState state;
-} ISASerialState;
-
 static void serial_receive1(void *opaque, const uint8_t *buf, int size);
 
 static void fifo_clear(SerialState *s, int fifo)
@@ -687,7 +625,7 @@ static int serial_post_load(void *opaque, int version_id)
     return 0;
 }
 
-static const VMStateDescription vmstate_serial = {
+const VMStateDescription vmstate_serial = {
     .name = "serial",
     .version_id = 3,
     .minimum_version_id = 2,
@@ -736,7 +674,7 @@ static void serial_reset(void *opaque)
     qemu_irq_lower(s->irq);
 }
 
-static void serial_init_core(SerialState *s)
+void serial_init_core(SerialState *s)
 {
     if (!s->chr) {
         fprintf(stderr, "Can't create serial device, empty char device\n");
@@ -761,54 +699,15 @@ void serial_set_frequency(SerialState *s, uint32_t frequency)
     serial_update_parameters(s);
 }
 
-static const int isa_serial_io[MAX_SERIAL_PORTS] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
-static const int isa_serial_irq[MAX_SERIAL_PORTS] = { 4, 3, 4, 3 };
-
 static const MemoryRegionPortio serial_portio[] = {
     { 0, 8, 1, .read = serial_ioport_read, .write = serial_ioport_write },
     PORTIO_END_OF_LIST()
 };
 
-static const MemoryRegionOps serial_io_ops = {
+const MemoryRegionOps serial_io_ops = {
     .old_portio = serial_portio
 };
 
-static int serial_isa_initfn(ISADevice *dev)
-{
-    static int index;
-    ISASerialState *isa = DO_UPCAST(ISASerialState, dev, dev);
-    SerialState *s = &isa->state;
-
-    if (isa->index == -1)
-        isa->index = index;
-    if (isa->index >= MAX_SERIAL_PORTS)
-        return -1;
-    if (isa->iobase == -1)
-        isa->iobase = isa_serial_io[isa->index];
-    if (isa->isairq == -1)
-        isa->isairq = isa_serial_irq[isa->index];
-    index++;
-
-    s->baudbase = 115200;
-    isa_init_irq(dev, &s->irq, isa->isairq);
-    serial_init_core(s);
-    qdev_set_legacy_instance_id(&dev->qdev, isa->iobase, 3);
-
-    memory_region_init_io(&s->io, &serial_io_ops, s, "serial", 8);
-    isa_register_ioport(dev, &s->io, isa->iobase);
-    return 0;
-}
-
-static const VMStateDescription vmstate_isa_serial = {
-    .name = "serial",
-    .version_id = 3,
-    .minimum_version_id = 2,
-    .fields      = (VMStateField []) {
-        VMSTATE_STRUCT(state, ISASerialState, 0, vmstate_serial, SerialState),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
 SerialState *serial_init(int base, qemu_irq irq, int baudbase,
                          CharDriverState *chr)
 {
@@ -886,35 +785,3 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
     serial_update_msl(s);
     return s;
 }
-
-static Property serial_isa_properties[] = {
-    DEFINE_PROP_UINT32("index", ISASerialState, index,   -1),
-    DEFINE_PROP_HEX32("iobase", ISASerialState, iobase,  -1),
-    DEFINE_PROP_UINT32("irq",   ISASerialState, isairq,  -1),
-    DEFINE_PROP_CHR("chardev",  ISASerialState, state.chr),
-    DEFINE_PROP_UINT32("wakeup", ISASerialState, state.wakeup, 0),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
-static void serial_isa_class_initfn(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-    ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
-    ic->init = serial_isa_initfn;
-    dc->vmsd = &vmstate_isa_serial;
-    dc->props = serial_isa_properties;
-}
-
-static TypeInfo serial_isa_info = {
-    .name          = "isa-serial",
-    .parent        = TYPE_ISA_DEVICE,
-    .instance_size = sizeof(ISASerialState),
-    .class_init    = serial_isa_class_initfn,
-};
-
-static void serial_register_types(void)
-{
-    type_register_static(&serial_isa_info);
-}
-
-type_init(serial_register_types)
diff --git a/hw/serial.h b/hw/serial.h
new file mode 100644
index 0000000..004a050
--- /dev/null
+++ b/hw/serial.h
@@ -0,0 +1,73 @@
+#include "hw.h"
+#include "sysemu.h"
+#include "memory.h"
+
+#define UART_FIFO_LENGTH    16      /* 16550A Fifo Length */
+
+typedef struct SerialFIFO {
+    uint8_t data[UART_FIFO_LENGTH];
+    uint8_t count;
+    uint8_t itl;                        /* Interrupt Trigger Level */
+    uint8_t tail;
+    uint8_t head;
+} SerialFIFO;
+
+struct SerialState {
+    uint16_t divider;
+    uint8_t rbr; /* receive register */
+    uint8_t thr; /* transmit holding register */
+    uint8_t tsr; /* transmit shift register */
+    uint8_t ier;
+    uint8_t iir; /* read only */
+    uint8_t lcr;
+    uint8_t mcr;
+    uint8_t lsr; /* read only */
+    uint8_t msr; /* read only */
+    uint8_t scr;
+    uint8_t fcr;
+    uint8_t fcr_vmstate; /* we can't write directly this value
+                            it has side effects */
+    /* NOTE: this hidden state is necessary for tx irq generation as
+       it can be reset while reading iir */
+    int thr_ipending;
+    qemu_irq irq;
+    CharDriverState *chr;
+    int last_break_enable;
+    int it_shift;
+    int baudbase;
+    int tsr_retry;
+    uint32_t wakeup;
+
+    /* Time when the last byte was successfully sent out of the tsr */
+    uint64_t last_xmit_ts;
+    SerialFIFO recv_fifo;
+    SerialFIFO xmit_fifo;
+
+    struct QEMUTimer *fifo_timeout_timer;
+    int timeout_ipending;           /* timeout interrupt pending state */
+    struct QEMUTimer *transmit_timer;
+
+
+    uint64_t char_transmit_time;    /* time to transmit a char in ticks */
+    int poll_msl;
+
+    struct QEMUTimer *modem_status_poll;
+    MemoryRegion io;
+};
+
+extern const VMStateDescription vmstate_serial;
+extern const MemoryRegionOps serial_io_ops;
+
+void serial_init_core(SerialState *s);
+void serial_set_frequency(SerialState *s, uint32_t frequency);
+
+/* legacy pre qom */
+SerialState *serial_init(int base, qemu_irq irq, int baudbase,
+                         CharDriverState *chr);
+SerialState *serial_mm_init(MemoryRegion *address_space,
+                            target_phys_addr_t base, int it_shift,
+                            qemu_irq irq, int baudbase,
+                            CharDriverState *chr, enum device_endian end);
+
+/* serial-isa.c */
+bool serial_isa_init(ISABus *bus, int index, CharDriverState *chr);
diff --git a/hw/sm501.c b/hw/sm501.c
index 786e076..050d096 100644
--- a/hw/sm501.c
+++ b/hw/sm501.c
@@ -24,7 +24,7 @@
 
 #include <stdio.h>
 #include "hw.h"
-#include "pc.h"
+#include "serial.h"
 #include "console.h"
 #include "devices.h"
 #include "sysbus.h"
diff --git a/hw/sun4u.c b/hw/sun4u.c
index 137a7c6..9151c8f 100644
--- a/hw/sun4u.c
+++ b/hw/sun4u.c
@@ -25,6 +25,7 @@
 #include "pci.h"
 #include "apb_pci.h"
 #include "pc.h"
+#include "serial.h"
 #include "nvram.h"
 #include "fdc.h"
 #include "net.h"
diff --git a/hw/virtex_ml507.c b/hw/virtex_ml507.c
index 79bc0d1..6ae5f60 100644
--- a/hw/virtex_ml507.c
+++ b/hw/virtex_ml507.c
@@ -24,7 +24,7 @@
 
 #include "sysbus.h"
 #include "hw.h"
-#include "pc.h"
+#include "serial.h"
 #include "net.h"
 #include "flash.h"
 #include "sysemu.h"
diff --git a/hw/xtensa_lx60.c b/hw/xtensa_lx60.c
index 3653f65..99d8b4f 100644
--- a/hw/xtensa_lx60.c
+++ b/hw/xtensa_lx60.c
@@ -31,7 +31,8 @@
 #include "elf.h"
 #include "memory.h"
 #include "exec-memory.h"
-#include "pc.h"
+#include "serial.h"
+#include "net.h"
 #include "sysbus.h"
 #include "flash.h"
 #include "blockdev.h"
-- 
1.7.1

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

* [Qemu-devel] [PATCH 2/7] serial: add pci variant
  2012-10-12  9:25 [Qemu-devel] [PULL 0/7] serial device hotplug patch series Gerd Hoffmann
  2012-10-12  9:25 ` [Qemu-devel] [PATCH 1/7] serial: split serial.c Gerd Hoffmann
@ 2012-10-12  9:25 ` Gerd Hoffmann
  2012-10-12  9:25 ` [Qemu-devel] [PATCH 3/7] serial: add windows inf file for the pci card to docs Gerd Hoffmann
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Gerd Hoffmann @ 2012-10-12  9:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

So we get a hot-pluggable 16550 uart.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 default-configs/pci.mak |    2 +
 hw/Makefile.objs        |    1 +
 hw/pci_ids.h            |    1 +
 hw/serial-pci.c         |  115 +++++++++++++++++++++++++++++++++++++++++++++++
 hw/serial.c             |    6 +++
 hw/serial.h             |    1 +
 6 files changed, 126 insertions(+), 0 deletions(-)
 create mode 100644 hw/serial-pci.c

diff --git a/default-configs/pci.mak b/default-configs/pci.mak
index 69e18f1..ae9d1eb 100644
--- a/default-configs/pci.mak
+++ b/default-configs/pci.mak
@@ -19,3 +19,5 @@ CONFIG_IDE_PCI=y
 CONFIG_AHCI=y
 CONFIG_ESP=y
 CONFIG_ESP_PCI=y
+CONFIG_SERIAL=y
+CONFIG_SERIAL_PCI=y
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 16e7a1e..af4ab0c 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -21,6 +21,7 @@ common-obj-$(CONFIG_ESCC) += escc.o
 common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
 
 common-obj-$(CONFIG_SERIAL) += serial.o serial-isa.o
+common-obj-$(CONFIG_SERIAL_PCI) += serial-pci.o
 common-obj-$(CONFIG_PARALLEL) += parallel.o
 common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
 common-obj-$(CONFIG_PCSPK) += pcspk.o
diff --git a/hw/pci_ids.h b/hw/pci_ids.h
index 301bf1c..c017a79 100644
--- a/hw/pci_ids.h
+++ b/hw/pci_ids.h
@@ -37,6 +37,7 @@
 #define PCI_CLASS_BRIDGE_PCI             0x0604
 #define PCI_CLASS_BRIDGE_OTHER           0x0680
 
+#define PCI_CLASS_COMMUNICATION_SERIAL   0x0700
 #define PCI_CLASS_COMMUNICATION_OTHER    0x0780
 
 #define PCI_CLASS_PROCESSOR_CO           0x0b40
diff --git a/hw/serial-pci.c b/hw/serial-pci.c
new file mode 100644
index 0000000..17247a8
--- /dev/null
+++ b/hw/serial-pci.c
@@ -0,0 +1,115 @@
+/*
+ * QEMU 16550A UART emulation
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ * Copyright (c) 2008 Citrix Systems, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+/*
+ * pci-serial spec:
+ *    pci region 0 is a io bar, 8 bytes long, with the 16550 uart mapped to it.
+ *    interrupt is wired to pin A.
+ *
+ * [root@fedora ~]# lspci -vnse
+ * 00:0e.0 0700: 1b36:0002 (rev 01) (prog-if 00 [8250])
+ *         Subsystem: 1af4:1100
+ *         Physical Slot: 14
+ *         Flags: fast devsel, IRQ 11
+ *         I/O ports at c130 [size=8]
+ *         Kernel driver in use: serial
+ */
+
+#include "serial.h"
+#include "pci.h"
+
+typedef struct PCISerialState {
+    PCIDevice dev;
+    SerialState state;
+} PCISerialState;
+
+static int serial_pci_init(PCIDevice *dev)
+{
+    PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
+    SerialState *s = &pci->state;
+
+    s->baudbase = 115200;
+    serial_init_core(s);
+
+    pci->dev.config[PCI_INTERRUPT_PIN] = 0x01;
+    s->irq = pci->dev.irq[0];
+
+    memory_region_init_io(&s->io, &serial_io_ops, s, "serial", 8);
+    pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io);
+    return 0;
+}
+
+static void serial_pci_exit(PCIDevice *dev)
+{
+    PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
+    SerialState *s = &pci->state;
+
+    serial_exit_core(s);
+    memory_region_destroy(&s->io);
+}
+
+static const VMStateDescription vmstate_pci_serial = {
+    .name = "pci-serial",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_PCI_DEVICE(dev, PCISerialState),
+        VMSTATE_STRUCT(state, PCISerialState, 0, vmstate_serial, SerialState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static Property serial_pci_properties[] = {
+    DEFINE_PROP_CHR("chardev",  PCISerialState, state.chr),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void serial_pci_class_initfn(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
+    pc->init = serial_pci_init;
+    pc->exit = serial_pci_exit;
+    pc->vendor_id = 0x1b36; /* Red Hat */
+    pc->device_id = 0x0002;
+    pc->revision = 1;
+    pc->class_id = PCI_CLASS_COMMUNICATION_SERIAL;
+    dc->vmsd = &vmstate_pci_serial;
+    dc->props = serial_pci_properties;
+}
+
+static TypeInfo serial_pci_info = {
+    .name          = "pci-serial",
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(PCISerialState),
+    .class_init    = serial_pci_class_initfn,
+};
+
+static void serial_pci_register_types(void)
+{
+    type_register_static(&serial_pci_info);
+}
+
+type_init(serial_pci_register_types)
diff --git a/hw/serial.c b/hw/serial.c
index 78e219d..5adbfaf 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -692,6 +692,12 @@ void serial_init_core(SerialState *s)
                           serial_event, s);
 }
 
+void serial_exit_core(SerialState *s)
+{
+    qemu_chr_add_handlers(s->chr, NULL, NULL, NULL, NULL);
+    qemu_unregister_reset(serial_reset, s);
+}
+
 /* Change the main reference oscillator frequency. */
 void serial_set_frequency(SerialState *s, uint32_t frequency)
 {
diff --git a/hw/serial.h b/hw/serial.h
index 004a050..8fbd992 100644
--- a/hw/serial.h
+++ b/hw/serial.h
@@ -59,6 +59,7 @@ extern const VMStateDescription vmstate_serial;
 extern const MemoryRegionOps serial_io_ops;
 
 void serial_init_core(SerialState *s);
+void serial_exit_core(SerialState *s);
 void serial_set_frequency(SerialState *s, uint32_t frequency);
 
 /* legacy pre qom */
-- 
1.7.1

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

* [Qemu-devel] [PATCH 3/7] serial: add windows inf file for the pci card to docs
  2012-10-12  9:25 [Qemu-devel] [PULL 0/7] serial device hotplug patch series Gerd Hoffmann
  2012-10-12  9:25 ` [Qemu-devel] [PATCH 1/7] serial: split serial.c Gerd Hoffmann
  2012-10-12  9:25 ` [Qemu-devel] [PATCH 2/7] serial: add pci variant Gerd Hoffmann
@ 2012-10-12  9:25 ` Gerd Hoffmann
  2012-10-12  9:25 ` [Qemu-devel] [PATCH 4/7] serial: add 2x + 4x pci variant Gerd Hoffmann
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Gerd Hoffmann @ 2012-10-12  9:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 docs/qemupciserial.inf |  107 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 107 insertions(+), 0 deletions(-)
 create mode 100644 docs/qemupciserial.inf

diff --git a/docs/qemupciserial.inf b/docs/qemupciserial.inf
new file mode 100644
index 0000000..c7cea99
--- /dev/null
+++ b/docs/qemupciserial.inf
@@ -0,0 +1,107 @@
+; qemupciserial.inf for QEMU, based on MSPORTS.INF
+
+; The driver itself is shipped with Windows (serial.sys).  This is
+; just a inf file to tell windows which pci id the serial pci card
+; emulated by qemu has, and to apply a name tag to it which windows
+; will show in the device manager.
+
+; Installing the driver: Go to device manager.  You should find a "pci
+; serial card" tagged with a yellow question mark.  Open properties.
+; Pick "update driver".  Then "select driver manually".  Pick "Ports
+; (Com+Lpt)" from the list.  Click "Have a disk".  Select this file.
+; Procedure may vary a bit depending on the windows version.
+
+[Version]
+Signature="$CHICAGO$"
+Class=Ports
+ClassGuid={4D36E978-E325-11CE-BFC1-08002BE10318}
+Provider=%QEMU%
+DriverVer=09/24/2012,1.3.0
+
+[SourceDisksNames]
+3426=windows cd
+
+[SourceDisksFiles]
+serial.sys 		= 3426
+serenum.sys 		= 3426
+
+[DestinationDirs]
+DefaultDestDir  = 11        ;LDID_SYS
+ComPort.NT.Copy = 12        ;DIRID_DRIVERS
+SerialEnumerator.NT.Copy=12 ;DIRID_DRIVERS
+
+; Drivers
+;----------------------------------------------------------
+[Manufacturer]
+%QEMU%=QEMU,NTx86
+
+[QEMU.NTx86]
+%QEMU-PCI_SERIAL.DeviceDesc% = ComPort, "PCI\VEN_1b36&DEV_0002&CC_0700"
+
+; COM sections
+;----------------------------------------------------------
+[ComPort.AddReg]
+HKR,,PortSubClass,1,01
+
+[ComPort.NT]
+AddReg=ComPort.AddReg, ComPort.NT.AddReg
+LogConfig=caa
+SyssetupPnPFlags = 1
+
+[ComPort.NT.HW]
+AddReg=ComPort.NT.HW.AddReg
+
+[ComPort.NT.AddReg]
+HKR,,EnumPropPages32,,"MsPorts.dll,SerialPortPropPageProvider"
+
+[ComPort.NT.HW.AddReg]
+HKR,,"UpperFilters",0x00010000,"serenum"
+
+;-------------- Service installation
+; Port Driver (function driver for this device)
+[ComPort.NT.Services]
+AddService = Serial, 0x00000002, Serial_Service_Inst, Serial_EventLog_Inst
+AddService = Serenum,,Serenum_Service_Inst
+
+; -------------- Serial Port Driver install sections
+[Serial_Service_Inst]
+DisplayName    = %Serial.SVCDESC%
+ServiceType    = 1               ; SERVICE_KERNEL_DRIVER
+StartType      = 1               ; SERVICE_SYSTEM_START (this driver may do detection)
+ErrorControl   = 0               ; SERVICE_ERROR_IGNORE
+ServiceBinary  = %12%\serial.sys
+LoadOrderGroup = Extended base
+
+; -------------- Serenum Driver install section
+[Serenum_Service_Inst]
+DisplayName    = %Serenum.SVCDESC%
+ServiceType    = 1               ; SERVICE_KERNEL_DRIVER
+StartType      = 3               ; SERVICE_DEMAND_START
+ErrorControl   = 1               ; SERVICE_ERROR_NORMAL
+ServiceBinary  = %12%\serenum.sys
+LoadOrderGroup = PNP Filter
+
+[Serial_EventLog_Inst]
+AddReg = Serial_EventLog_AddReg
+
+[Serial_EventLog_AddReg]
+HKR,,EventMessageFile,0x00020000,"%%SystemRoot%%\System32\IoLogMsg.dll;%%SystemRoot%%\System32\drivers\serial.sys"
+HKR,,TypesSupported,0x00010001,7
+
+; The following sections are COM port resource configs.
+; Section name format means:
+; Char 1 = c (COM port)
+; Char 2 = I/O config: 1 (3f8), 2 (2f8), 3 (3e8), 4 (2e8), a (any)
+; Char 3 = IRQ config: #, a (any)
+
+[caa]                   ; Any base, any IRQ
+ConfigPriority=HARDRECONFIG
+IOConfig=8@100-ffff%fff8(3ff::)
+IRQConfig=S:3,4,5,7,9,10,11,12,14,15
+
+[Strings]
+QEMU="QEMU"
+QEMU-PCI_SERIAL.DeviceDesc="QEMU Serial PCI Card"
+
+Serial.SVCDESC   = "Serial port driver"
+Serenum.SVCDESC = "Serenum Filter Driver"
-- 
1.7.1

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

* [Qemu-devel] [PATCH 4/7] serial: add 2x + 4x pci variant
  2012-10-12  9:25 [Qemu-devel] [PULL 0/7] serial device hotplug patch series Gerd Hoffmann
                   ` (2 preceding siblings ...)
  2012-10-12  9:25 ` [Qemu-devel] [PATCH 3/7] serial: add windows inf file for the pci card to docs Gerd Hoffmann
@ 2012-10-12  9:25 ` Gerd Hoffmann
  2012-10-12  9:26 ` [Qemu-devel] [PATCH 5/7] usb-serial: don't magically zap chardev on umplug Gerd Hoffmann
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Gerd Hoffmann @ 2012-10-12  9:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Add multiport serial card implementation, with two variants,
one featuring two and one featuring four ports.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 docs/qemupciserial.inf |    2 +
 hw/serial-pci.c        |  157 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 159 insertions(+), 0 deletions(-)

diff --git a/docs/qemupciserial.inf b/docs/qemupciserial.inf
index c7cea99..3474310 100644
--- a/docs/qemupciserial.inf
+++ b/docs/qemupciserial.inf
@@ -11,6 +11,8 @@
 ; (Com+Lpt)" from the list.  Click "Have a disk".  Select this file.
 ; Procedure may vary a bit depending on the windows version.
 
+; FIXME: This file covers the single port version only.
+
 [Version]
 Signature="$CHICAGO$"
 Class=Ports
diff --git a/hw/serial-pci.c b/hw/serial-pci.c
index 17247a8..c89e8b0 100644
--- a/hw/serial-pci.c
+++ b/hw/serial-pci.c
@@ -28,6 +28,14 @@
  *    pci region 0 is a io bar, 8 bytes long, with the 16550 uart mapped to it.
  *    interrupt is wired to pin A.
  *
+ * pci-serial-4x spec:
+ *    pci region 0 is a io bar, with four 16550 uarts mapped after each other,
+ *    the first at offset 0, second at 8, third at 16 and fourth at 24.
+ *    interrupt is wired to pin A.
+ *
+ * pci-serial-2x spec:
+ *    same as pci-serial-4x but with two uarts only.
+ *
  * [root@fedora ~]# lspci -vnse
  * 00:0e.0 0700: 1b36:0002 (rev 01) (prog-if 00 [8250])
  *         Subsystem: 1af4:1100
@@ -40,11 +48,23 @@
 #include "serial.h"
 #include "pci.h"
 
+#define PCI_SERIAL_MAX_PORTS 4
+
 typedef struct PCISerialState {
     PCIDevice dev;
     SerialState state;
 } PCISerialState;
 
+typedef struct PCIMultiSerialState {
+    PCIDevice    dev;
+    MemoryRegion iobar;
+    uint32_t     ports;
+    char         *name[PCI_SERIAL_MAX_PORTS];
+    SerialState  state[PCI_SERIAL_MAX_PORTS];
+    uint32_t     level[PCI_SERIAL_MAX_PORTS];
+    qemu_irq     *irqs;
+} PCIMultiSerialState;
+
 static int serial_pci_init(PCIDevice *dev)
 {
     PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
@@ -61,6 +81,56 @@ static int serial_pci_init(PCIDevice *dev)
     return 0;
 }
 
+static void multi_serial_irq_mux(void *opaque, int n, int level)
+{
+    PCIMultiSerialState *pci = opaque;
+    int i, pending = 0;
+
+    pci->level[n] = level;
+    for (i = 0; i < pci->ports; i++) {
+        if (pci->level[i]) {
+            pending = 1;
+        }
+    }
+    qemu_set_irq(pci->dev.irq[0], pending);
+}
+
+static int multi_serial_pci_init(PCIDevice *dev)
+{
+    PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
+    PCIMultiSerialState *pci = DO_UPCAST(PCIMultiSerialState, dev, dev);
+    SerialState *s;
+    int i;
+
+    switch (pc->device_id) {
+    case 0x0003:
+        pci->ports = 2;
+        break;
+    case 0x0004:
+        pci->ports = 4;
+        break;
+    }
+    assert(pci->ports > 0);
+    assert(pci->ports <= PCI_SERIAL_MAX_PORTS);
+
+    pci->dev.config[PCI_INTERRUPT_PIN] = 0x01;
+    memory_region_init(&pci->iobar, "multiserial", 8 * pci->ports);
+    pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->iobar);
+    pci->irqs = qemu_allocate_irqs(multi_serial_irq_mux, pci,
+                                   pci->ports);
+
+    for (i = 0; i < pci->ports; i++) {
+        s = pci->state + i;
+        s->baudbase = 115200;
+        serial_init_core(s);
+        s->irq = pci->irqs[i];
+        pci->name[i] = g_strdup_printf("uart #%d", i+1);
+        memory_region_init_io(&s->io, &serial_io_ops, s, pci->name[i], 8);
+        memory_region_add_subregion(&pci->iobar, 8 * i, &s->io);
+    }
+    return 0;
+}
+
 static void serial_pci_exit(PCIDevice *dev)
 {
     PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
@@ -70,6 +140,22 @@ static void serial_pci_exit(PCIDevice *dev)
     memory_region_destroy(&s->io);
 }
 
+static void multi_serial_pci_exit(PCIDevice *dev)
+{
+    PCIMultiSerialState *pci = DO_UPCAST(PCIMultiSerialState, dev, dev);
+    SerialState *s;
+    int i;
+
+    for (i = 0; i < pci->ports; i++) {
+        s = pci->state + i;
+        serial_exit_core(s);
+        memory_region_destroy(&s->io);
+        g_free(pci->name[i]);
+    }
+    memory_region_destroy(&pci->iobar);
+    qemu_free_irqs(pci->irqs);
+}
+
 static const VMStateDescription vmstate_pci_serial = {
     .name = "pci-serial",
     .version_id = 1,
@@ -81,11 +167,38 @@ static const VMStateDescription vmstate_pci_serial = {
     }
 };
 
+static const VMStateDescription vmstate_pci_multi_serial = {
+    .name = "pci-serial-multi",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_PCI_DEVICE(dev, PCIMultiSerialState),
+        VMSTATE_STRUCT_ARRAY(state, PCIMultiSerialState, PCI_SERIAL_MAX_PORTS,
+                             0, vmstate_serial, SerialState),
+        VMSTATE_UINT32_ARRAY(level, PCIMultiSerialState, PCI_SERIAL_MAX_PORTS),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static Property serial_pci_properties[] = {
     DEFINE_PROP_CHR("chardev",  PCISerialState, state.chr),
     DEFINE_PROP_END_OF_LIST(),
 };
 
+static Property multi_2x_serial_pci_properties[] = {
+    DEFINE_PROP_CHR("chardev1",  PCIMultiSerialState, state[0].chr),
+    DEFINE_PROP_CHR("chardev2",  PCIMultiSerialState, state[1].chr),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static Property multi_4x_serial_pci_properties[] = {
+    DEFINE_PROP_CHR("chardev1",  PCIMultiSerialState, state[0].chr),
+    DEFINE_PROP_CHR("chardev2",  PCIMultiSerialState, state[1].chr),
+    DEFINE_PROP_CHR("chardev3",  PCIMultiSerialState, state[2].chr),
+    DEFINE_PROP_CHR("chardev4",  PCIMultiSerialState, state[3].chr),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void serial_pci_class_initfn(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -100,6 +213,34 @@ static void serial_pci_class_initfn(ObjectClass *klass, void *data)
     dc->props = serial_pci_properties;
 }
 
+static void multi_2x_serial_pci_class_initfn(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
+    pc->init = multi_serial_pci_init;
+    pc->exit = multi_serial_pci_exit;
+    pc->vendor_id = 0x1b36; /* Red Hat */
+    pc->device_id = 0x0003;
+    pc->revision = 1;
+    pc->class_id = PCI_CLASS_COMMUNICATION_SERIAL;
+    dc->vmsd = &vmstate_pci_multi_serial;
+    dc->props = multi_2x_serial_pci_properties;
+}
+
+static void multi_4x_serial_pci_class_initfn(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
+    pc->init = multi_serial_pci_init;
+    pc->exit = multi_serial_pci_exit;
+    pc->vendor_id = 0x1b36; /* Red Hat */
+    pc->device_id = 0x0004;
+    pc->revision = 1;
+    pc->class_id = PCI_CLASS_COMMUNICATION_SERIAL;
+    dc->vmsd = &vmstate_pci_multi_serial;
+    dc->props = multi_4x_serial_pci_properties;
+}
+
 static TypeInfo serial_pci_info = {
     .name          = "pci-serial",
     .parent        = TYPE_PCI_DEVICE,
@@ -107,9 +248,25 @@ static TypeInfo serial_pci_info = {
     .class_init    = serial_pci_class_initfn,
 };
 
+static TypeInfo multi_2x_serial_pci_info = {
+    .name          = "pci-serial-2x",
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(PCIMultiSerialState),
+    .class_init    = multi_2x_serial_pci_class_initfn,
+};
+
+static TypeInfo multi_4x_serial_pci_info = {
+    .name          = "pci-serial-4x",
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(PCIMultiSerialState),
+    .class_init    = multi_4x_serial_pci_class_initfn,
+};
+
 static void serial_pci_register_types(void)
 {
     type_register_static(&serial_pci_info);
+    type_register_static(&multi_2x_serial_pci_info);
+    type_register_static(&multi_4x_serial_pci_info);
 }
 
 type_init(serial_pci_register_types)
-- 
1.7.1

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

* [Qemu-devel] [PATCH 5/7] usb-serial: don't magically zap chardev on umplug
  2012-10-12  9:25 [Qemu-devel] [PULL 0/7] serial device hotplug patch series Gerd Hoffmann
                   ` (3 preceding siblings ...)
  2012-10-12  9:25 ` [Qemu-devel] [PATCH 4/7] serial: add 2x + 4x pci variant Gerd Hoffmann
@ 2012-10-12  9:26 ` Gerd Hoffmann
  2012-10-12  9:26 ` [Qemu-devel] [PATCH 6/7] usb-serial: only expose device in guest when the chardev is open Gerd Hoffmann
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Gerd Hoffmann @ 2012-10-12  9:26 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-serial.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index 69b6e48..43214cd 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -421,7 +421,7 @@ static void usb_serial_handle_destroy(USBDevice *dev)
 {
     USBSerialState *s = (USBSerialState *)dev;
 
-    qemu_chr_delete(s->cs);
+    qemu_chr_add_handlers(s->cs, NULL, NULL, NULL, NULL);
 }
 
 static int usb_serial_can_read(void *opaque)
-- 
1.7.1

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

* [Qemu-devel] [PATCH 6/7] usb-serial: only expose device in guest when the chardev is open
  2012-10-12  9:25 [Qemu-devel] [PULL 0/7] serial device hotplug patch series Gerd Hoffmann
                   ` (4 preceding siblings ...)
  2012-10-12  9:26 ` [Qemu-devel] [PATCH 5/7] usb-serial: don't magically zap chardev on umplug Gerd Hoffmann
@ 2012-10-12  9:26 ` Gerd Hoffmann
  2012-10-12  9:26 ` [Qemu-devel] [PATCH 7/7] chardev: add hotplug support Gerd Hoffmann
  2012-10-12  9:42 ` [Qemu-devel] [PULL 0/7] serial device hotplug patch series Paolo Bonzini
  7 siblings, 0 replies; 24+ messages in thread
From: Gerd Hoffmann @ 2012-10-12  9:26 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/dev-serial.c |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index 43214cd..a466f99 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -427,6 +427,10 @@ static void usb_serial_handle_destroy(USBDevice *dev)
 static int usb_serial_can_read(void *opaque)
 {
     USBSerialState *s = opaque;
+
+    if (!s->dev.attached) {
+        return 0;
+    }
     return RECV_BUF - s->recv_used;
 }
 
@@ -469,8 +473,14 @@ static void usb_serial_event(void *opaque, int event)
         case CHR_EVENT_FOCUS:
             break;
         case CHR_EVENT_OPENED:
-            usb_serial_reset(s);
-            /* TODO: Reset USB port */
+            if (!s->dev.attached) {
+                usb_device_attach(&s->dev);
+            }
+            break;
+        case CHR_EVENT_CLOSED:
+            if (s->dev.attached) {
+                usb_device_detach(&s->dev);
+            }
             break;
     }
 }
@@ -481,6 +491,7 @@ static int usb_serial_initfn(USBDevice *dev)
 
     usb_desc_create_serial(dev);
     usb_desc_init(dev);
+    dev->auto_attach = 0;
 
     if (!s->cs) {
         error_report("Property chardev is required");
@@ -490,6 +501,10 @@ static int usb_serial_initfn(USBDevice *dev)
     qemu_chr_add_handlers(s->cs, usb_serial_can_read, usb_serial_read,
                           usb_serial_event, s);
     usb_serial_handle_reset(dev);
+
+    if (s->cs->opened && !dev->attached) {
+        usb_device_attach(dev);
+    }
     return 0;
 }
 
-- 
1.7.1

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

* [Qemu-devel] [PATCH 7/7] chardev: add hotplug support.
  2012-10-12  9:25 [Qemu-devel] [PULL 0/7] serial device hotplug patch series Gerd Hoffmann
                   ` (5 preceding siblings ...)
  2012-10-12  9:26 ` [Qemu-devel] [PATCH 6/7] usb-serial: only expose device in guest when the chardev is open Gerd Hoffmann
@ 2012-10-12  9:26 ` Gerd Hoffmann
  2012-10-12  9:40   ` Paolo Bonzini
  2012-10-12  9:42 ` [Qemu-devel] [PULL 0/7] serial device hotplug patch series Paolo Bonzini
  7 siblings, 1 reply; 24+ messages in thread
From: Gerd Hoffmann @ 2012-10-12  9:26 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

This patch adds chardev_add and chardev_del monitor commands.

chardev_del is pretty straight forward, it just takes an id argument and
zaps the chardev specified.

chardev_add is more tricky as there are tons of arguments for the
different backends.  The hmp version limited to the most common use
cases, especially when it comes to sockets:  You can only specify port
(tcp) or path (unix) and qemu will create a listening socket.  For
example this ...

   (qemu) chardev_add foo socket 42

... will do the same as ...

   -chardev socket,id=foo,port=42,server,nowait

on the qemu command line.

The qmp version has full support for everything the -chardev command
line switch can handle.  The implementation is pretty straight
forward: It just puts all arguments it got into a QemuOpts, then goes
call qemu_chr_new_from_opts().

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hmp-commands.hx  |   49 +++++++++++++++++++++++++++++++++++
 hmp.c            |   39 ++++++++++++++++++++++++++++
 hmp.h            |    2 +
 qapi-schema.json |   46 +++++++++++++++++++++++++++++++++
 qemu-char.c      |   74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qmp-commands.hx  |   61 ++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 271 insertions(+), 0 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e0b537d..e5590f4 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1404,6 +1404,55 @@ passed since 1970, i.e. unix epoch.
 ETEXI
 
     {
+        .name       = "chardev_add",
+        .args_type  = "id:s,backend:s,arg:s?",
+        .params     = "id backend [ name | port | path ]",
+        .help       = "add chardev",
+        .mhandler.cmd = hmp_chardev_add,
+    },
+
+STEXI
+@item chardev_add id backend [ name | port | path ]
+@findex chardev_add
+
+Create chardev with the specified @var{id} and @var{backend}.  The hmp
+version is limited to a commonly used subset, if you need more control
+use qmp instead.
+
+If @var{backend} is 'spicevmc' @var{arg} is assumed to be the spice
+channel name.
+
+If @var{backend} is 'udp' @var{arg} is assumed to be a port number.
+
+If @var{backend} is 'socket' and @var{arg} starts with a digit the
+argument is assumed to be a port number and a tcp socket is created
+(in server mode).
+
+If @var{backend} is 'socket' and @var{arg} doesn't start with a digit
+@var{arg} is assumed to be a path and a unix socket is created (in
+server mode).
+
+In all other cases @var{arg} is assumed to be a path.
+
+ETEXI
+
+    {
+        .name       = "chardev_del",
+        .args_type  = "id:s",
+        .params     = "id",
+        .help       = "del chardev",
+        .mhandler.cmd = hmp_chardev_del,
+    },
+
+STEXI
+@item chardev_del id
+@findex chardev_del
+
+Removes the chardev @var{id}.
+
+ETEXI
+
+    {
         .name       = "info",
         .args_type  = "item:s?",
         .params     = "[subcommand]",
diff --git a/hmp.c b/hmp.c
index 70bdec2..e84d7f8 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1209,3 +1209,42 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
     qmp_screendump(filename, &err);
     hmp_handle_error(mon, &err);
 }
+
+void hmp_chardev_add(Monitor *mon, const QDict *qdict)
+{
+    const char *backend = qdict_get_str(qdict, "backend");
+    const char *arg = qdict_get_str(qdict, "arg");
+    const char *id = qdict_get_str(qdict, "id");
+    const char *path = NULL, *name = NULL, *port = NULL, *host = NULL;
+    Error *err = NULL;
+
+    if (arg) {
+        if (strcmp(backend, "spicevmc") == 0) {
+            name = arg;
+        } else if (strcmp(backend, "udp") == 0) {
+            port = arg;
+        } else if (strcmp(backend, "socket") == 0 && isdigit(arg[0])) {
+            port = arg;
+        } else {
+            path = arg;
+        }
+    }
+
+    qmp_chardev_add(id, backend,
+                    path, name, host, port,
+                    true,  /* server      */
+                    false, /* wait        */
+                    false, /* ipv4 (only) */
+                    false, /* ipv6 (only) */
+                    false, /* telnet      */
+                    &err);
+    hmp_handle_error(mon, &err);
+}
+
+void hmp_chardev_del(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    qmp_chardev_del(qdict_get_str(qdict, "id"),
+                    &err);
+    hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index 71ea384..080afaa 100644
--- a/hmp.h
+++ b/hmp.h
@@ -75,5 +75,7 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
 void hmp_closefd(Monitor *mon, const QDict *qdict);
 void hmp_send_key(Monitor *mon, const QDict *qdict);
 void hmp_screen_dump(Monitor *mon, const QDict *qdict);
+void hmp_chardev_add(Monitor *mon, const QDict *qdict);
+void hmp_chardev_del(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index f9dbdae..98d92ad 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2796,3 +2796,49 @@
 # Since: 0.14.0
 ##
 { 'command': 'screendump', 'data': {'filename': 'str'} }
+
+##
+# @chardev_add:
+#
+# Add a chardev
+#
+# @id: the chardev's ID, must be unique
+# @backend: the chardev backend: "file", "socket", ...
+# @path: file / device / unix socket path
+# @name: spice channel name
+# @host: host name
+# @port: port number
+# @server: create socket in server mode
+# @wait: wait for connect
+# @ipv4: force ipv4-only
+# @ipv6: force ipv6-only
+# @telnet: telnet negotiation
+#
+# Returns: Nothing on success
+#
+# Since: 1.3.0
+##
+{ 'command': 'chardev_add', 'data': {'id'      : 'str',
+                                     'backend' : 'str',
+                                     'path'    : 'str',
+                                     'name'    : 'str',
+                                     'host'    : 'str',
+                                     'port'    : 'str',
+                                     'server'  : 'bool',
+                                     'wait'    : 'bool',
+                                     'ipv4'    : 'bool',
+                                     'ipv6'    : 'bool',
+                                     'telnet'  : 'bool' } }
+
+##
+# @chardev_del:
+#
+# Remove a chardev
+#
+# @id: the chardev's ID, must exist and not be in use
+#
+# Returns: Nothing on success
+#
+# Since: 1.3.0
+##
+{ 'command': 'chardev_del', 'data': {'id': 'str'} }
diff --git a/qemu-char.c b/qemu-char.c
index b082bae..2f9d860 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2900,3 +2900,77 @@ CharDriverState *qemu_char_get_next_serial(void)
     return serial_hds[next_serial++];
 }
 
+void qmp_chardev_add(const char *id, const char *backend,
+                     const char *path, const char *name,
+                     const char *host, const char *port,
+                     bool server, bool wait,
+                     bool ipv4, bool ipv6,
+                     bool telnet, Error **errp)
+{
+    CharDriverState *chr;
+    QemuOpts *opts;
+
+    chr = qemu_chr_find(id);
+    if (NULL != chr) {
+        error_setg(errp, "Chardev id '%s' exists already\n", id);
+        return;
+    }
+
+    opts = qemu_opts_create(qemu_find_opts("chardev"), id, 1, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+    qemu_opt_set(opts, "backend", backend);
+    if (path) {
+        qemu_opt_set(opts, "path", path);
+    }
+    if (name) {
+        qemu_opt_set(opts, "name", name);
+    }
+    if (host) {
+        qemu_opt_set(opts, "host", host);
+    }
+    if (port) {
+        qemu_opt_set(opts, "port", port);
+    }
+    if (server) {
+        qemu_opt_set(opts, "server", "on");
+    }
+    if (!wait) {
+        qemu_opt_set(opts, "wait", "off");
+    }
+    if (ipv4) {
+        qemu_opt_set(opts, "ipv4", "on");
+    }
+    if (ipv6) {
+        qemu_opt_set(opts, "ipv6", "on");
+    }
+    if (telnet) {
+        qemu_opt_set(opts, "telnet", "on");
+    }
+
+    chr = qemu_chr_new_from_opts(opts, NULL);
+    qemu_opts_del(opts);
+
+    if (chr == NULL) {
+        error_setg(errp, "Creating chardev failed\n");
+        return;
+    }
+}
+
+void qmp_chardev_del(const char *id, Error **errp)
+{
+    CharDriverState *chr;
+
+    chr = qemu_chr_find(id);
+    if (NULL == chr) {
+        error_setg(errp, "Chardev '%s' not found\n", id);
+        return;
+    }
+    if (chr->chr_can_read || chr->chr_read ||
+        chr->chr_event || chr->handler_opaque) {
+        error_setg(errp, "Chardev '%s' is busy\n", id);
+        return;
+    }
+    qemu_chr_delete(chr);
+}
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 2f8477e..b904df2 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2576,3 +2576,64 @@ EQMP
         .args_type  = "",
         .mhandler.cmd_new = qmp_marshal_input_query_target,
     },
+
+    {
+        .name       = "chardev_add",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_chardev_add,
+    },
+
+SQMP
+chardev_add
+-----------
+
+Add a chardev.
+
+Arguments:
+
+- "id": the chardev's ID, must be unique (json-string)
+- "backend": the chardev backend: "file", "socket", ... (json-string)
+- "path": file / device / unix socket path (json-string, optional)
+- "name": spice channel name (json-string, optional)
+- "host": host name (json-string, optional)
+- "port": port number (json-string, optional)
+- "server": create socket in server mode (json-bool, optional)
+- "wait": wait for connect (json-bool, optional)
+- "ipv4": force ipv4-only (json-bool, optional)
+- "ipv6": force ipv6-only (json-bool, optional)
+- "telnet": telnet negotiation (json-bool, optional)
+
+Example:
+
+-> { "execute": "chardev_add", "arguments": { "id"      : "foo",
+                                              "backend" : "socket",
+                                              "path"    : "/tmp/foo",
+                                              "server"  : "on",
+                                              "wait"    : "off" } }
+<- { "return": {} }
+
+EQMP
+
+    {
+        .name       = "chardev_del",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_chardev_del,
+    },
+
+
+SQMP
+chardev_del
+-----------
+
+Remove a chardev.
+
+Arguments:
+
+- "id": the chardev's ID, must exist and not be in use (json-string)
+
+Example:
+
+-> { "execute": "chardev_del", "arguments": { "id" : "foo" } }
+<- { "return": {} }
+
+EQMP
-- 
1.7.1

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

* Re: [Qemu-devel] [PATCH 7/7] chardev: add hotplug support.
  2012-10-12  9:26 ` [Qemu-devel] [PATCH 7/7] chardev: add hotplug support Gerd Hoffmann
@ 2012-10-12  9:40   ` Paolo Bonzini
  2012-10-12 10:23     ` Gerd Hoffmann
  0 siblings, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2012-10-12  9:40 UTC (permalink / raw)
  To: qemu-devel

Il 12/10/2012 11:26, Gerd Hoffmann ha scritto:
> This patch adds chardev_add and chardev_del monitor commands.
> 
> chardev_del is pretty straight forward, it just takes an id argument and
> zaps the chardev specified.
> 
> chardev_add is more tricky as there are tons of arguments for the
> different backends.  The hmp version limited to the most common use
> cases, especially when it comes to sockets:  You can only specify port
> (tcp) or path (unix) and qemu will create a listening socket.  For
> example this ...
> 
>    (qemu) chardev_add foo socket 42
> 
> ... will do the same as ...
> 
>    -chardev socket,id=foo,port=42,server,nowait

Why not

chardev_add socket,id=foo,port=42,server,nowait

?

> +{ 'command': 'chardev_add', 'data': {'id'      : 'str',
> +                                     'backend' : 'str',
> +                                     'path'    : 'str',
> +                                     'name'    : 'str',
> +                                     'host'    : 'str',
> +                                     'port'    : 'str',

You cannot pass NULLs via QMP, so these need to be optional.

I suggest that you implement the commands in a similar way as netdev_add.

Paolo

> +                                     'server'  : 'bool',
> +                                     'wait'    : 'bool',
> +                                     'ipv4'    : 'bool',
> +                                     'ipv6'    : 'bool',
> +                                     'telnet'  : 'bool' } }
> +
> +##
> +# @chardev_del:
> +#
> +# Remove a chardev
> +#
> +# @id: the chardev's ID, must exist and not be in use
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 1.3.0
> +##
> +{ 'command': 'chardev_del', 'data': {'id': 'str'} }
> diff --git a/qemu-char.c b/qemu-char.c
> index b082bae..2f9d860 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2900,3 +2900,77 @@ CharDriverState *qemu_char_get_next_serial(void)
>      return serial_hds[next_serial++];
>  }
>  
> +void qmp_chardev_add(const char *id, const char *backend,
> +                     const char *path, const char *name,
> +                     const char *host, const char *port,
> +                     bool server, bool wait,
> +                     bool ipv4, bool ipv6,
> +                     bool telnet, Error **errp)
> +{
> +    CharDriverState *chr;
> +    QemuOpts *opts;
> +
> +    chr = qemu_chr_find(id);
> +    if (NULL != chr) {
> +        error_setg(errp, "Chardev id '%s' exists already\n", id);
> +        return;
> +    }
> +
> +    opts = qemu_opts_create(qemu_find_opts("chardev"), id, 1, errp);
> +    if (error_is_set(errp)) {
> +        return;
> +    }
> +    qemu_opt_set(opts, "backend", backend);
> +    if (path) {
> +        qemu_opt_set(opts, "path", path);
> +    }
> +    if (name) {
> +        qemu_opt_set(opts, "name", name);
> +    }
> +    if (host) {
> +        qemu_opt_set(opts, "host", host);
> +    }
> +    if (port) {
> +        qemu_opt_set(opts, "port", port);
> +    }
> +    if (server) {
> +        qemu_opt_set(opts, "server", "on");
> +    }
> +    if (!wait) {
> +        qemu_opt_set(opts, "wait", "off");
> +    }
> +    if (ipv4) {
> +        qemu_opt_set(opts, "ipv4", "on");
> +    }
> +    if (ipv6) {
> +        qemu_opt_set(opts, "ipv6", "on");
> +    }
> +    if (telnet) {
> +        qemu_opt_set(opts, "telnet", "on");
> +    }
> +
> +    chr = qemu_chr_new_from_opts(opts, NULL);
> +    qemu_opts_del(opts);
> +
> +    if (chr == NULL) {
> +        error_setg(errp, "Creating chardev failed\n");
> +        return;
> +    }
> +}
> +
> +void qmp_chardev_del(const char *id, Error **errp)
> +{
> +    CharDriverState *chr;
> +
> +    chr = qemu_chr_find(id);
> +    if (NULL == chr) {
> +        error_setg(errp, "Chardev '%s' not found\n", id);
> +        return;
> +    }
> +    if (chr->chr_can_read || chr->chr_read ||
> +        chr->chr_event || chr->handler_opaque) {
> +        error_setg(errp, "Chardev '%s' is busy\n", id);
> +        return;
> +    }
> +    qemu_chr_delete(chr);
> +}
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 2f8477e..b904df2 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2576,3 +2576,64 @@ EQMP
>          .args_type  = "",
>          .mhandler.cmd_new = qmp_marshal_input_query_target,
>      },
> +
> +    {
> +        .name       = "chardev_add",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_input_chardev_add,
> +    },
> +
> +SQMP
> +chardev_add
> +-----------
> +
> +Add a chardev.
> +
> +Arguments:
> +
> +- "id": the chardev's ID, must be unique (json-string)
> +- "backend": the chardev backend: "file", "socket", ... (json-string)
> +- "path": file / device / unix socket path (json-string, optional)
> +- "name": spice channel name (json-string, optional)
> +- "host": host name (json-string, optional)
> +- "port": port number (json-string, optional)
> +- "server": create socket in server mode (json-bool, optional)
> +- "wait": wait for connect (json-bool, optional)
> +- "ipv4": force ipv4-only (json-bool, optional)
> +- "ipv6": force ipv6-only (json-bool, optional)
> +- "telnet": telnet negotiation (json-bool, optional)
> +
> +Example:
> +
> +-> { "execute": "chardev_add", "arguments": { "id"      : "foo",
> +                                              "backend" : "socket",
> +                                              "path"    : "/tmp/foo",
> +                                              "server"  : "on",
> +                                              "wait"    : "off" } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +    {
> +        .name       = "chardev_del",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_input_chardev_del,
> +    },
> +
> +
> +SQMP
> +chardev_del
> +-----------
> +
> +Remove a chardev.
> +
> +Arguments:
> +
> +- "id": the chardev's ID, must exist and not be in use (json-string)
> +
> +Example:
> +
> +-> { "execute": "chardev_del", "arguments": { "id" : "foo" } }
> +<- { "return": {} }
> +
> +EQMP
> 

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

* Re: [Qemu-devel] [PULL 0/7] serial device hotplug patch series.
  2012-10-12  9:25 [Qemu-devel] [PULL 0/7] serial device hotplug patch series Gerd Hoffmann
                   ` (6 preceding siblings ...)
  2012-10-12  9:26 ` [Qemu-devel] [PATCH 7/7] chardev: add hotplug support Gerd Hoffmann
@ 2012-10-12  9:42 ` Paolo Bonzini
  7 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2012-10-12  9:42 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel, Luiz Capitulino

Il 12/10/2012 11:25, Gerd Hoffmann ha scritto:
> Final patch adds chardev hotplug to the mix, which makes the other
> patches alot more useful.  It is the missing bit needed to really
> hotplug serial devices:
> 
>    (qemu) chardev_add pciserial file /root/hotchardev.log
>    (qemu) device_add pci-serial,chardev=pciserial,id=pciserial
> 
> And the reverse:
> 
>    (qemu) device_del pciserial
>    (qemu) chardev_del pciserial
> 
> please review & pull,

I think the last patch is not ready, please wait for Luiz's review too.

>   Gerd
> 
> The following changes since commit b4ae3cfa57b8c1bdbbd7b7d420971e9171203ade:
> 
>   ssi: Add slave autoconnect helper (2012-10-10 11:13:32 +1000)
> 
> are available in the git repository at:
>   git://git.kraxel.org/qemu serial.1
> 
> Gerd Hoffmann (7):
>       serial: split serial.c
>       serial: add pci variant
>       serial: add windows inf file for the pci card to docs
>       serial: add 2x + 4x pci variant
>       usb-serial: don't magically zap chardev on umplug
>       usb-serial: only expose device in guest when the chardev is open
>       chardev: add hotplug support.

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

* Re: [Qemu-devel] [PATCH 7/7] chardev: add hotplug support.
  2012-10-12  9:40   ` Paolo Bonzini
@ 2012-10-12 10:23     ` Gerd Hoffmann
  2012-10-12 10:50       ` Paolo Bonzini
  0 siblings, 1 reply; 24+ messages in thread
From: Gerd Hoffmann @ 2012-10-12 10:23 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

On 10/12/12 11:40, Paolo Bonzini wrote:
> Il 12/10/2012 11:26, Gerd Hoffmann ha scritto:
>> This patch adds chardev_add and chardev_del monitor commands.
>>
>> chardev_del is pretty straight forward, it just takes an id argument and
>> zaps the chardev specified.
>>
>> chardev_add is more tricky as there are tons of arguments for the
>> different backends.  The hmp version limited to the most common use
>> cases, especially when it comes to sockets:  You can only specify port
>> (tcp) or path (unix) and qemu will create a listening socket.  For
>> example this ...
>>
>>    (qemu) chardev_add foo socket 42
>>
>> ... will do the same as ...
>>
>>    -chardev socket,id=foo,port=42,server,nowait
> 
> Why not
> 
> chardev_add socket,id=foo,port=42,server,nowait
> 
> ?

Yea, maybe, but see below.

>> +{ 'command': 'chardev_add', 'data': {'id'      : 'str',
>> +                                     'backend' : 'str',
>> +                                     'path'    : 'str',
>> +                                     'name'    : 'str',
>> +                                     'host'    : 'str',
>> +                                     'port'    : 'str',
> 
> You cannot pass NULLs via QMP, so these need to be optional.

Fixed.

> I suggest that you implement the commands in a similar way as netdev_add.

Why?  Isn't the whole point of using josn is that you'll get the stuff
from the josn parser & marshaller in a usable form instead of having it
to feed into yet another parser?  I think the only reason netdev_add
exists in the current form is that it predates qmp.

cheers,
  Gerd

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

* Re: [Qemu-devel] [PATCH 7/7] chardev: add hotplug support.
  2012-10-12 10:23     ` Gerd Hoffmann
@ 2012-10-12 10:50       ` Paolo Bonzini
  2012-10-12 11:15         ` Gerd Hoffmann
  0 siblings, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2012-10-12 10:50 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

Il 12/10/2012 12:23, Gerd Hoffmann ha scritto:
>> I suggest that you implement the commands in a similar way as netdev_add.
> 
> Why?  Isn't the whole point of using josn is that you'll get the stuff
> from the josn parser & marshaller in a usable form instead of having it
> to feed into yet another parser?  I think the only reason netdev_add
> exists in the current form is that it predates qmp.

In principle you're right, but I think it's ugly that adding another
chardev argument needs changes in 3 places instead of just one.  (And
I'd like to add another argument soon enough...).

Paolo

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

* Re: [Qemu-devel] [PATCH 7/7] chardev: add hotplug support.
  2012-10-12 10:50       ` Paolo Bonzini
@ 2012-10-12 11:15         ` Gerd Hoffmann
  2012-10-12 11:17           ` Paolo Bonzini
  0 siblings, 1 reply; 24+ messages in thread
From: Gerd Hoffmann @ 2012-10-12 11:15 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

On 10/12/12 12:50, Paolo Bonzini wrote:
> Il 12/10/2012 12:23, Gerd Hoffmann ha scritto:
>>> I suggest that you implement the commands in a similar way as netdev_add.
>>
>> Why?  Isn't the whole point of using josn is that you'll get the stuff
>> from the josn parser & marshaller in a usable form instead of having it
>> to feed into yet another parser?  I think the only reason netdev_add
>> exists in the current form is that it predates qmp.
> 
> In principle you're right, but I think it's ugly that adding another
> chardev argument needs changes in 3 places instead of just one.

Hmm, I don't have to use the generated marshaller, right?  With direct
access to the QDict I could just transform it into a QemuOpts.  A new
parameter wouldn't need code changes then.  And the code would be
reusable and probably also be simpler.  The qapi schema still needs an
update though.

HMP is more tricky, but I think we should sort QMP first.

cheers,
  Gerd

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

* Re: [Qemu-devel] [PATCH 7/7] chardev: add hotplug support.
  2012-10-12 11:15         ` Gerd Hoffmann
@ 2012-10-12 11:17           ` Paolo Bonzini
  2012-10-12 12:37             ` Gerd Hoffmann
  0 siblings, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2012-10-12 11:17 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

Il 12/10/2012 13:15, Gerd Hoffmann ha scritto:
>> > In principle you're right, but I think it's ugly that adding another
>> > chardev argument needs changes in 3 places instead of just one.
> Hmm, I don't have to use the generated marshaller, right?  With direct
> access to the QDict I could just transform it into a QemuOpts.

That's exactly what I was suggesting. :P

> A new parameter wouldn't need code changes then.  And the code would be 
> reusable and probably also be simpler.  The qapi schema still needs
> an update though.

The QAPI schema is only used by the generated marshaller.

> HMP is more tricky, but I think we should sort QMP first.

HMP can just take a string, parse it into QemuOpts, and call a small
wrapper that calls qemu_chr_new_from_opts and returns errors via Error.

Paolo

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

* Re: [Qemu-devel] [PATCH 7/7] chardev: add hotplug support.
  2012-10-12 11:17           ` Paolo Bonzini
@ 2012-10-12 12:37             ` Gerd Hoffmann
  2012-10-12 12:39               ` [Qemu-devel] [PATCH v2] " Gerd Hoffmann
  0 siblings, 1 reply; 24+ messages in thread
From: Gerd Hoffmann @ 2012-10-12 12:37 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

On 10/12/12 13:17, Paolo Bonzini wrote:
> Il 12/10/2012 13:15, Gerd Hoffmann ha scritto:
>>>> In principle you're right, but I think it's ugly that adding another
>>>> chardev argument needs changes in 3 places instead of just one.
>> Hmm, I don't have to use the generated marshaller, right?  With direct
>> access to the QDict I could just transform it into a QemuOpts.
> 
> That's exactly what I was suggesting. :P

Ah, ok.  I actually looked at netdev_add but obviously not close
enougth.  On a quick glance it looked to me like @params is a single
string, not a varargs-style construct.  Guess we are on the same page then.

cheers,
  Gerd

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

* [Qemu-devel] [PATCH v2] chardev: add hotplug support.
  2012-10-12 12:37             ` Gerd Hoffmann
@ 2012-10-12 12:39               ` Gerd Hoffmann
  2012-10-12 13:12                 ` Gerd Hoffmann
                                   ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Gerd Hoffmann @ 2012-10-12 12:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

This patch adds chardev_add and chardev_del monitor commands.

chardev_del is pretty straight forward, it just takes an id argument and
zaps the chardev specified.

chardev_add is more tricky as there are tons of arguments for the
different backends.  The hmp version limited to the most common use
cases, especially when it comes to sockets:  You can only specify port
(tcp) or path (unix) and qemu will create a listening socket.  For
example this ...

   (qemu) chardev_add foo socket 42

... will do the same as ...

   -chardev socket,id=foo,port=42,server,nowait

on the qemu command line.

The qmp version has full support for everything the -chardev command
line switch can handle.  The implementation is pretty straight
forward: It just puts all arguments it got into a QemuOpts, then goes
call qemu_chr_new_from_opts().

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hmp-commands.hx  |   32 ++++++++++++++++++++++++++++
 hmp.c            |   31 +++++++++++++++++++++++++++
 hmp.h            |    2 +
 qapi-schema.json |   39 ++++++++++++++++++++++++++++++++++
 qemu-char.c      |   50 +++++++++++++++++++++++++++++++++++++++++++-
 qemu-char.h      |    2 +
 qmp-commands.hx  |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 216 insertions(+), 1 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e0b537d..48504d1 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1404,6 +1404,38 @@ passed since 1970, i.e. unix epoch.
 ETEXI
 
     {
+        .name       = "chardev_add",
+        .args_type  = "args:s",
+        .params     = "args",
+        .help       = "add chardev",
+        .mhandler.cmd = hmp_chardev_add,
+    },
+
+STEXI
+@item chardev_add args
+@findex chardev_add
+
+chardev_add accepts the same parameters as the -chardev command line switch.
+
+ETEXI
+
+    {
+        .name       = "chardev_del",
+        .args_type  = "id:s",
+        .params     = "id",
+        .help       = "del chardev",
+        .mhandler.cmd = hmp_chardev_del,
+    },
+
+STEXI
+@item chardev_del id
+@findex chardev_del
+
+Removes the chardev @var{id}.
+
+ETEXI
+
+    {
         .name       = "info",
         .args_type  = "item:s?",
         .params     = "[subcommand]",
diff --git a/hmp.c b/hmp.c
index 70bdec2..b494d05 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1209,3 +1209,34 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
     qmp_screendump(filename, &err);
     hmp_handle_error(mon, &err);
 }
+
+void hmp_chardev_add(Monitor *mon, const QDict *qdict)
+{
+    const char *args = qdict_get_str(qdict, "args");
+    CharDriverState *chr;
+    Error *err = NULL;
+    QemuOpts *opts;
+
+    opts = qemu_opts_parse(qemu_find_opts("chardev"), args, 1);
+    if (opts == NULL) {
+        error_setg(&err, "Parsing chardev args failed\n");
+        goto out;
+    }
+
+    chr = qemu_chr_new_from_opts(opts, NULL);
+    if (chr == NULL) {
+        qemu_opts_del(opts);
+        error_setg(&err, "Creating chardev failed\n");
+    }
+
+out:
+    hmp_handle_error(mon, &err);
+}
+
+void hmp_chardev_del(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    qmp_chardev_del(qdict_get_str(qdict, "id"),
+                    &err);
+    hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index 71ea384..080afaa 100644
--- a/hmp.h
+++ b/hmp.h
@@ -75,5 +75,7 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
 void hmp_closefd(Monitor *mon, const QDict *qdict);
 void hmp_send_key(Monitor *mon, const QDict *qdict);
 void hmp_screen_dump(Monitor *mon, const QDict *qdict);
+void hmp_chardev_add(Monitor *mon, const QDict *qdict);
+void hmp_chardev_del(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index f9dbdae..550e4c7 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2796,3 +2796,42 @@
 # Since: 0.14.0
 ##
 { 'command': 'screendump', 'data': {'filename': 'str'} }
+
+##
+# @chardev_add:
+#
+# Add a chardev
+#
+# @id: the chardev's ID, must be unique
+# @backend: the chardev backend: "file", "socket", ...
+# @path: file / device / unix socket path
+# @name: spice channel name
+# @host: host name
+# @port: port number
+# @server: create socket in server mode
+# @wait: wait for connect
+# @ipv4: force ipv4-only
+# @ipv6: force ipv6-only
+# @telnet: telnet negotiation
+#
+# Returns: Nothing on success
+#
+# Since: 1.3.0
+##
+{ 'command': 'chardev_add', 'data': {'id'      : 'str',
+                                     'backend' : 'str',
+                                     '*props'  : '**' },
+  'gen': 'no' }
+
+##
+# @chardev_del:
+#
+# Remove a chardev
+#
+# @id: the chardev's ID, must exist and not be in use
+#
+# Returns: Nothing on success
+#
+# Since: 1.3.0
+##
+{ 'command': 'chardev_del', 'data': {'id': 'str'} }
diff --git a/qemu-char.c b/qemu-char.c
index b082bae..7bbc490 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2805,6 +2805,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
         chr->avail_connections = 1;
     }
     chr->label = g_strdup(qemu_opts_id(opts));
+    chr->opts = opts;
     return chr;
 }
 
@@ -2826,7 +2827,6 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in
     if (chr && qemu_opt_get_bool(opts, "mux", 0)) {
         monitor_init(chr, MONITOR_USE_READLINE);
     }
-    qemu_opts_del(opts);
     return chr;
 }
 
@@ -2856,6 +2856,7 @@ void qemu_chr_delete(CharDriverState *chr)
     QTAILQ_REMOVE(&chardevs, chr, next);
     if (chr->chr_close)
         chr->chr_close(chr);
+    qemu_opts_del(chr->opts);
     g_free(chr->filename);
     g_free(chr->label);
     g_free(chr);
@@ -2900,3 +2901,50 @@ CharDriverState *qemu_char_get_next_serial(void)
     return serial_hds[next_serial++];
 }
 
+int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret)
+{
+    CharDriverState *chr;
+    Error *err = NULL;
+    QemuOptsList *opts_list;
+    QemuOpts *opts;
+
+    opts_list = qemu_find_opts_err("chardev", &err);
+    if (error_is_set(&err)) {
+        goto exit_err;
+    }
+
+    opts = qemu_opts_from_qdict(opts_list, qdict, &err);
+    if (error_is_set(&err)) {
+        goto exit_err;
+    }
+
+    chr = qemu_chr_new_from_opts(opts, NULL);
+    if (chr == NULL) {
+        qemu_opts_del(opts);
+        error_setg(&err, "Creating chardev failed\n");
+        goto exit_err;
+    }
+    return 0;
+
+exit_err:
+    qerror_report_err(err);
+    error_free(err);
+    return -1;
+}
+
+void qmp_chardev_del(const char *id, Error **errp)
+{
+    CharDriverState *chr;
+
+    chr = qemu_chr_find(id);
+    if (NULL == chr) {
+        error_setg(errp, "Chardev '%s' not found\n", id);
+        return;
+    }
+    if (chr->chr_can_read || chr->chr_read ||
+        chr->chr_event || chr->handler_opaque) {
+        error_setg(errp, "Chardev '%s' is busy\n", id);
+        return;
+    }
+    qemu_chr_delete(chr);
+}
diff --git a/qemu-char.h b/qemu-char.h
index 486644b..a910a60 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -74,6 +74,7 @@ struct CharDriverState {
     char *filename;
     int opened;
     int avail_connections;
+    QemuOpts *opts;
     QTAILQ_ENTRY(CharDriverState) next;
 };
 
@@ -236,6 +237,7 @@ void qemu_chr_info(Monitor *mon, QObject **ret_data);
 CharDriverState *qemu_chr_find(const char *name);
 
 QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
+int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret);
 
 /* add an eventfd to the qemu devices that are polled */
 CharDriverState *qemu_chr_open_eventfd(int eventfd);
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 2f8477e..ebc1cee 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2576,3 +2576,64 @@ EQMP
         .args_type  = "",
         .mhandler.cmd_new = qmp_marshal_input_query_target,
     },
+
+    {
+        .name       = "chardev_add",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_chardev_add,
+    },
+
+SQMP
+chardev_add
+-----------
+
+Add a chardev.
+
+Arguments:
+
+- "id": the chardev's ID, must be unique (json-string)
+- "backend": the chardev backend: "file", "socket", ... (json-string)
+- "path": file / device / unix socket path (json-string, optional)
+- "name": spice channel name (json-string, optional)
+- "host": host name (json-string, optional)
+- "port": port number (json-string, optional)
+- "server": create socket in server mode (json-bool, optional)
+- "wait": wait for connect (json-bool, optional)
+- "ipv4": force ipv4-only (json-bool, optional)
+- "ipv6": force ipv6-only (json-bool, optional)
+- "telnet": telnet negotiation (json-bool, optional)
+
+Example:
+
+-> { "execute": "chardev_add", "arguments": { "id"      : "foo",
+                                              "backend" : "socket",
+                                              "path"    : "/tmp/foo",
+                                              "server"  : "on",
+                                              "wait"    : "off" } }
+<- { "return": {} }
+
+EQMP
+
+    {
+        .name       = "chardev_del",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_chardev_del,
+    },
+
+
+SQMP
+chardev_del
+-----------
+
+Remove a chardev.
+
+Arguments:
+
+- "id": the chardev's ID, must exist and not be in use (json-string)
+
+Example:
+
+-> { "execute": "chardev_del", "arguments": { "id" : "foo" } }
+<- { "return": {} }
+
+EQMP
-- 
1.7.1

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

* Re: [Qemu-devel] [PATCH v2] chardev: add hotplug support.
  2012-10-12 12:39               ` [Qemu-devel] [PATCH v2] " Gerd Hoffmann
@ 2012-10-12 13:12                 ` Gerd Hoffmann
  2012-10-12 16:54                 ` Paolo Bonzini
  2012-10-15  6:51                 ` Lei Li
  2 siblings, 0 replies; 24+ messages in thread
From: Gerd Hoffmann @ 2012-10-12 13:12 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

On 10/12/12 14:39, Gerd Hoffmann wrote:
> This patch adds chardev_add and chardev_del monitor commands.
> 
> chardev_del is pretty straight forward, it just takes an id argument and
> zaps the chardev specified.
> 
> chardev_add is more tricky as there are tons of arguments for the

Oops.  Ignore the commit message for now, it obviously needs to be
updated to reflect the changes, will fix for v3, awaiting code reviews
meanwhile.

cheers,
  Gerd

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

* Re: [Qemu-devel] [PATCH v2] chardev: add hotplug support.
  2012-10-12 12:39               ` [Qemu-devel] [PATCH v2] " Gerd Hoffmann
  2012-10-12 13:12                 ` Gerd Hoffmann
@ 2012-10-12 16:54                 ` Paolo Bonzini
  2012-10-12 17:08                   ` Paolo Bonzini
  2012-10-15  6:51                 ` Lei Li
  2 siblings, 1 reply; 24+ messages in thread
From: Paolo Bonzini @ 2012-10-12 16:54 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

Il 12/10/2012 14:39, Gerd Hoffmann ha scritto:
> +    chr = qemu_chr_new_from_opts(opts, NULL);
> +    if (chr == NULL) {
> +        qemu_opts_del(opts);
> +        error_setg(&err, "Creating chardev failed\n");
> +        goto exit_err;
> +    }
> +    return 0;
> +

Since you have to do a v3, please make this part (which is common
between HMP and QMP) a separate function.

Paolo

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

* Re: [Qemu-devel] [PATCH v2] chardev: add hotplug support.
  2012-10-12 16:54                 ` Paolo Bonzini
@ 2012-10-12 17:08                   ` Paolo Bonzini
  0 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2012-10-12 17:08 UTC (permalink / raw)
  Cc: Gerd Hoffmann, qemu-devel

Il 12/10/2012 18:54, Paolo Bonzini ha scritto:
> Il 12/10/2012 14:39, Gerd Hoffmann ha scritto:
>> +    chr = qemu_chr_new_from_opts(opts, NULL);
>> +    if (chr == NULL) {
>> +        qemu_opts_del(opts);
>> +        error_setg(&err, "Creating chardev failed\n");
>> +        goto exit_err;
>> +    }
>> +    return 0;
>> +
> 
> Since you have to do a v3, please make this part (which is common
> between HMP and QMP) a separate function.

Also please make qemu_chr_new_from_opts use an Error ** at least for the
error that are reported from that function itself (no need to add
propagation to all of qemu-char.c, and it would conflict badly with the
series that does so for qemu-sockets.c).

Paolo

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

* Re: [Qemu-devel] [PATCH v2] chardev: add hotplug support.
  2012-10-12 12:39               ` [Qemu-devel] [PATCH v2] " Gerd Hoffmann
  2012-10-12 13:12                 ` Gerd Hoffmann
  2012-10-12 16:54                 ` Paolo Bonzini
@ 2012-10-15  6:51                 ` Lei Li
  2012-10-15 11:09                   ` Andreas Färber
  2012-10-15 17:36                   ` Eric Blake
  2 siblings, 2 replies; 24+ messages in thread
From: Lei Li @ 2012-10-15  6:51 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

On 10/12/2012 08:39 PM, Gerd Hoffmann wrote:
> This patch adds chardev_add and chardev_del monitor commands.
>
> chardev_del is pretty straight forward, it just takes an id argument and
> zaps the chardev specified.
>
> chardev_add is more tricky as there are tons of arguments for the
> different backends.  The hmp version limited to the most common use
> cases, especially when it comes to sockets:  You can only specify port
> (tcp) or path (unix) and qemu will create a listening socket.  For
> example this ...
>
>     (qemu) chardev_add foo socket 42
>
> ... will do the same as ...
>
>     -chardev socket,id=foo,port=42,server,nowait
>
> on the qemu command line.
>
> The qmp version has full support for everything the -chardev command
> line switch can handle.  The implementation is pretty straight
> forward: It just puts all arguments it got into a QemuOpts, then goes
> call qemu_chr_new_from_opts().
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   hmp-commands.hx  |   32 ++++++++++++++++++++++++++++
>   hmp.c            |   31 +++++++++++++++++++++++++++
>   hmp.h            |    2 +
>   qapi-schema.json |   39 ++++++++++++++++++++++++++++++++++
>   qemu-char.c      |   50 +++++++++++++++++++++++++++++++++++++++++++-
>   qemu-char.h      |    2 +
>   qmp-commands.hx  |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   7 files changed, 216 insertions(+), 1 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e0b537d..48504d1 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1404,6 +1404,38 @@ passed since 1970, i.e. unix epoch.
>   ETEXI
>
>       {
> +        .name       = "chardev_add",
> +        .args_type  = "args:s",
> +        .params     = "args",
> +        .help       = "add chardev",
> +        .mhandler.cmd = hmp_chardev_add,
> +    },
> +
> +STEXI
> +@item chardev_add args
> +@findex chardev_add
> +
> +chardev_add accepts the same parameters as the -chardev command line switch.
> +
> +ETEXI
> +
> +    {
> +        .name       = "chardev_del",
> +        .args_type  = "id:s",
> +        .params     = "id",
> +        .help       = "del chardev",
> +        .mhandler.cmd = hmp_chardev_del,
> +    },
> +
> +STEXI
> +@item chardev_del id
> +@findex chardev_del
> +
> +Removes the chardev @var{id}.
> +
> +ETEXI
> +
> +    {
>           .name       = "info",
>           .args_type  = "item:s?",
>           .params     = "[subcommand]",
> diff --git a/hmp.c b/hmp.c
> index 70bdec2..b494d05 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1209,3 +1209,34 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
>       qmp_screendump(filename, &err);
>       hmp_handle_error(mon, &err);
>   }
> +
> +void hmp_chardev_add(Monitor *mon, const QDict *qdict)
> +{
> +    const char *args = qdict_get_str(qdict, "args");
> +    CharDriverState *chr;
> +    Error *err = NULL;
> +    QemuOpts *opts;
> +
> +    opts = qemu_opts_parse(qemu_find_opts("chardev"), args, 1);
> +    if (opts == NULL) {
> +        error_setg(&err, "Parsing chardev args failed\n");
> +        goto out;
> +    }
> +
> +    chr = qemu_chr_new_from_opts(opts, NULL);
> +    if (chr == NULL) {
> +        qemu_opts_del(opts);
> +        error_setg(&err, "Creating chardev failed\n");
> +    }
> +
> +out:
> +    hmp_handle_error(mon, &err);
> +}
> +
> +void hmp_chardev_del(Monitor *mon, const QDict *qdict)
> +{
> +    Error *err = NULL;
> +    qmp_chardev_del(qdict_get_str(qdict, "id"),
> +                    &err);
> +    hmp_handle_error(mon, &err);
> +}
> diff --git a/hmp.h b/hmp.h
> index 71ea384..080afaa 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -75,5 +75,7 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
>   void hmp_closefd(Monitor *mon, const QDict *qdict);
>   void hmp_send_key(Monitor *mon, const QDict *qdict);
>   void hmp_screen_dump(Monitor *mon, const QDict *qdict);
> +void hmp_chardev_add(Monitor *mon, const QDict *qdict);
> +void hmp_chardev_del(Monitor *mon, const QDict *qdict);
>
>   #endif
> diff --git a/qapi-schema.json b/qapi-schema.json
> index f9dbdae..550e4c7 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2796,3 +2796,42 @@
>   # Since: 0.14.0
>   ##
>   { 'command': 'screendump', 'data': {'filename': 'str'} }
> +
> +##
> +# @chardev_add:
> +#
> +# Add a chardev
> +#
> +# @id: the chardev's ID, must be unique
> +# @backend: the chardev backend: "file", "socket", ...
> +# @path: file / device / unix socket path
> +# @name: spice channel name
> +# @host: host name
> +# @port: port number
> +# @server: create socket in server mode
> +# @wait: wait for connect
> +# @ipv4: force ipv4-only
> +# @ipv6: force ipv6-only
> +# @telnet: telnet negotiation
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 1.3.0
> +##
> +{ 'command': 'chardev_add', 'data': {'id'      : 'str',
> +                                     'backend' : 'str',
> +                                     '*props'  : '**' },
> +  'gen': 'no' }
> +
> +##
> +# @chardev_del:
> +#
> +# Remove a chardev
> +#
> +# @id: the chardev's ID, must exist and not be in use
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 1.3.0
> +##
> +{ 'command': 'chardev_del', 'data': {'id': 'str'} }
> diff --git a/qemu-char.c b/qemu-char.c
> index b082bae..7bbc490 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2805,6 +2805,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
>           chr->avail_connections = 1;
>       }
>       chr->label = g_strdup(qemu_opts_id(opts));
> +    chr->opts = opts;
>       return chr;
>   }
>
> @@ -2826,7 +2827,6 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in
>       if (chr && qemu_opt_get_bool(opts, "mux", 0)) {
>           monitor_init(chr, MONITOR_USE_READLINE);
>       }
> -    qemu_opts_del(opts);
>       return chr;
>   }
>
> @@ -2856,6 +2856,7 @@ void qemu_chr_delete(CharDriverState *chr)
>       QTAILQ_REMOVE(&chardevs, chr, next);
>       if (chr->chr_close)
>           chr->chr_close(chr);
> +    qemu_opts_del(chr->opts);
>       g_free(chr->filename);
>       g_free(chr->label);
>       g_free(chr);
> @@ -2900,3 +2901,50 @@ CharDriverState *qemu_char_get_next_serial(void)
>       return serial_hds[next_serial++];
>   }
>
> +int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret)
> +{
> +    CharDriverState *chr;
> +    Error *err = NULL;
> +    QemuOptsList *opts_list;
> +    QemuOpts *opts;
> +
> +    opts_list = qemu_find_opts_err("chardev", &err);
> +    if (error_is_set(&err)) {
> +        goto exit_err;
> +    }
> +
> +    opts = qemu_opts_from_qdict(opts_list, qdict, &err);
> +    if (error_is_set(&err)) {
> +        goto exit_err;
> +    }
> +
> +    chr = qemu_chr_new_from_opts(opts, NULL);
> +    if (chr == NULL) {
> +        qemu_opts_del(opts);
> +        error_setg(&err, "Creating chardev failed\n");
> +        goto exit_err;
> +    }
> +    return 0;
> +
> +exit_err:
> +    qerror_report_err(err);
> +    error_free(err);
> +    return -1;
> +}
> +
> +void qmp_chardev_del(const char *id, Error **errp)
> +{
> +    CharDriverState *chr;
> +
> +    chr = qemu_chr_find(id);
> +    if (NULL == chr) {
> +        error_setg(errp, "Chardev '%s' not found\n", id);

Maybe this should be replaced by QERR_ macros to keep
compatibility since this one is listed in ErrorClass
in the schema, like:

error_set(errp, QERR_DEVICE_NOT_FOUND, id);

> +        return;
> +    }
> +    if (chr->chr_can_read || chr->chr_read ||
> +        chr->chr_event || chr->handler_opaque) {
> +        error_setg(errp, "Chardev '%s' is busy\n", id);
> +        return;
> +    }
> +    qemu_chr_delete(chr);
> +}
> diff --git a/qemu-char.h b/qemu-char.h
> index 486644b..a910a60 100644
> --- a/qemu-char.h
> +++ b/qemu-char.h
> @@ -74,6 +74,7 @@ struct CharDriverState {
>       char *filename;
>       int opened;
>       int avail_connections;
> +    QemuOpts *opts;
>       QTAILQ_ENTRY(CharDriverState) next;
>   };
>
> @@ -236,6 +237,7 @@ void qemu_chr_info(Monitor *mon, QObject **ret_data);
>   CharDriverState *qemu_chr_find(const char *name);
>
>   QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
> +int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret);
>
>   /* add an eventfd to the qemu devices that are polled */
>   CharDriverState *qemu_chr_open_eventfd(int eventfd);
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 2f8477e..ebc1cee 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2576,3 +2576,64 @@ EQMP
>           .args_type  = "",
>           .mhandler.cmd_new = qmp_marshal_input_query_target,
>       },
> +
> +    {
> +        .name       = "chardev_add",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_chardev_add,
> +    },
> +
> +SQMP
> +chardev_add
> +-----------
> +
> +Add a chardev.
> +
> +Arguments:
> +
> +- "id": the chardev's ID, must be unique (json-string)
> +- "backend": the chardev backend: "file", "socket", ... (json-string)
> +- "path": file / device / unix socket path (json-string, optional)
> +- "name": spice channel name (json-string, optional)
> +- "host": host name (json-string, optional)
> +- "port": port number (json-string, optional)
> +- "server": create socket in server mode (json-bool, optional)
> +- "wait": wait for connect (json-bool, optional)
> +- "ipv4": force ipv4-only (json-bool, optional)
> +- "ipv6": force ipv6-only (json-bool, optional)
> +- "telnet": telnet negotiation (json-bool, optional)
> +

Should it support the option "mux" to enable multiplexing mode?

> +Example:
> +
> +-> { "execute": "chardev_add", "arguments": { "id"      : "foo",
> +                                              "backend" : "socket",
> +                                              "path"    : "/tmp/foo",
> +                                              "server"  : "on",
> +                                              "wait"    : "off" } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +    {
> +        .name       = "chardev_del",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_input_chardev_del,
> +    },
> +
> +
> +SQMP
> +chardev_del
> +-----------
> +
> +Remove a chardev.
> +
> +Arguments:
> +
> +- "id": the chardev's ID, must exist and not be in use (json-string)
> +
> +Example:
> +
> +-> { "execute": "chardev_del", "arguments": { "id" : "foo" } }
> +<- { "return": {} }
> +
> +EQMP


-- 
Lei

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

* Re: [Qemu-devel] [PATCH v2] chardev: add hotplug support.
  2012-10-15  6:51                 ` Lei Li
@ 2012-10-15 11:09                   ` Andreas Färber
  2012-10-16 13:23                     ` Luiz Capitulino
  2012-10-15 17:36                   ` Eric Blake
  1 sibling, 1 reply; 24+ messages in thread
From: Andreas Färber @ 2012-10-15 11:09 UTC (permalink / raw)
  To: Lei Li; +Cc: Luiz Capitulino, Gerd Hoffmann, qemu-devel

Am 15.10.2012 08:51, schrieb Lei Li:
> On 10/12/2012 08:39 PM, Gerd Hoffmann wrote:
>> +void qmp_chardev_del(const char *id, Error **errp)
>> +{
>> +    CharDriverState *chr;
>> +
>> +    chr = qemu_chr_find(id);
>> +    if (NULL == chr) {
>> +        error_setg(errp, "Chardev '%s' not found\n", id);
> 
> Maybe this should be replaced by QERR_ macros to keep
> compatibility since this one is listed in ErrorClass
> in the schema, like:
> 
> error_set(errp, QERR_DEVICE_NOT_FOUND, id);

No, error_setg() is the new replacement, QERR_* is deprecated.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

* Re: [Qemu-devel] [PATCH v2] chardev: add hotplug support.
  2012-10-15  6:51                 ` Lei Li
  2012-10-15 11:09                   ` Andreas Färber
@ 2012-10-15 17:36                   ` Eric Blake
  2012-10-16 14:02                     ` Paolo Bonzini
  1 sibling, 1 reply; 24+ messages in thread
From: Eric Blake @ 2012-10-15 17:36 UTC (permalink / raw)
  To: Lei Li; +Cc: Gerd Hoffmann, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1793 bytes --]

On 10/15/2012 12:51 AM, Lei Li wrote:
> On 10/12/2012 08:39 PM, Gerd Hoffmann wrote:
>> This patch adds chardev_add and chardev_del monitor commands.
>>
>> chardev_del is pretty straight forward, it just takes an id argument and
>> zaps the chardev specified.
>>
>> chardev_add is more tricky as there are tons of arguments for the
>> different backends.  The hmp version limited to the most common use
>> cases, especially when it comes to sockets:  You can only specify port
>> (tcp) or path (unix) and qemu will create a listening socket.  For
>> example this ...
>>
>>     (qemu) chardev_add foo socket 42
>>
>> ... will do the same as ...
>>
>>     -chardev socket,id=foo,port=42,server,nowait
>>
>> on the qemu command line.
>>
>> The qmp version has full support for everything the -chardev command
>> line switch can handle.  The implementation is pretty straight
>> forward: It just puts all arguments it got into a QemuOpts, then goes
>> call qemu_chr_new_from_opts().
>>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

>> +++ b/qapi-schema.json
>> @@ -2796,3 +2796,42 @@
>>   # Since: 0.14.0
>>   ##
>>   { 'command': 'screendump', 'data': {'filename': 'str'} }
>> +
>> +##
>> +# @chardev_add:

The QMP command should be named 'chardev-add'.

>> +#
>> +# Add a chardev
>> +#
>> +# @id: the chardev's ID, must be unique
>> +# @backend: the chardev backend: "file", "socket", ...

Rather than making this an open-coded string, should it instead be a QMP
enum value?

>> +##
>> +# @chardev_del:

And this should be 'chardev-del' or even 'chardev-remove', as QMP
commands tend to favor legibility over abbreviations.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 617 bytes --]

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

* Re: [Qemu-devel] [PATCH v2] chardev: add hotplug support.
  2012-10-15 11:09                   ` Andreas Färber
@ 2012-10-16 13:23                     ` Luiz Capitulino
  0 siblings, 0 replies; 24+ messages in thread
From: Luiz Capitulino @ 2012-10-16 13:23 UTC (permalink / raw)
  To: Andreas Färber; +Cc: qemu-devel, Lei Li, Gerd Hoffmann

On Mon, 15 Oct 2012 13:09:55 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 15.10.2012 08:51, schrieb Lei Li:
> > On 10/12/2012 08:39 PM, Gerd Hoffmann wrote:
> >> +void qmp_chardev_del(const char *id, Error **errp)
> >> +{
> >> +    CharDriverState *chr;
> >> +
> >> +    chr = qemu_chr_find(id);
> >> +    if (NULL == chr) {
> >> +        error_setg(errp, "Chardev '%s' not found\n", id);
> > 
> > Maybe this should be replaced by QERR_ macros to keep
> > compatibility since this one is listed in ErrorClass
> > in the schema, like:
> > 
> > error_set(errp, QERR_DEVICE_NOT_FOUND, id);
> 
> No, error_setg() is the new replacement, QERR_* is deprecated.

This is a case where the error appears in the ErrorClass enum, in this
case we're supposed to use the QERR_ macro for compatibility in the wire.

On the other hand, this is a new command so I'm not completely sure
this matters (in doubt we should do it though).

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

* Re: [Qemu-devel] [PATCH v2] chardev: add hotplug support.
  2012-10-15 17:36                   ` Eric Blake
@ 2012-10-16 14:02                     ` Paolo Bonzini
  0 siblings, 0 replies; 24+ messages in thread
From: Paolo Bonzini @ 2012-10-16 14:02 UTC (permalink / raw)
  To: Eric Blake; +Cc: qemu-devel, Lei Li, Gerd Hoffmann

Il 15/10/2012 19:36, Eric Blake ha scritto:
>>> >> +##
>>> >> +# @chardev_del:
> And this should be 'chardev-del' or even 'chardev-remove', as QMP
> commands tend to favor legibility over abbreviations.

In doubt we should favor consistency though (with netdev_add and
netdev_del).  I'm for keeping this as Gerd did, and perhaps implement
automatic _ <-> - conversion for QMP.

Paolo

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

end of thread, other threads:[~2012-10-16 14:02 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-12  9:25 [Qemu-devel] [PULL 0/7] serial device hotplug patch series Gerd Hoffmann
2012-10-12  9:25 ` [Qemu-devel] [PATCH 1/7] serial: split serial.c Gerd Hoffmann
2012-10-12  9:25 ` [Qemu-devel] [PATCH 2/7] serial: add pci variant Gerd Hoffmann
2012-10-12  9:25 ` [Qemu-devel] [PATCH 3/7] serial: add windows inf file for the pci card to docs Gerd Hoffmann
2012-10-12  9:25 ` [Qemu-devel] [PATCH 4/7] serial: add 2x + 4x pci variant Gerd Hoffmann
2012-10-12  9:26 ` [Qemu-devel] [PATCH 5/7] usb-serial: don't magically zap chardev on umplug Gerd Hoffmann
2012-10-12  9:26 ` [Qemu-devel] [PATCH 6/7] usb-serial: only expose device in guest when the chardev is open Gerd Hoffmann
2012-10-12  9:26 ` [Qemu-devel] [PATCH 7/7] chardev: add hotplug support Gerd Hoffmann
2012-10-12  9:40   ` Paolo Bonzini
2012-10-12 10:23     ` Gerd Hoffmann
2012-10-12 10:50       ` Paolo Bonzini
2012-10-12 11:15         ` Gerd Hoffmann
2012-10-12 11:17           ` Paolo Bonzini
2012-10-12 12:37             ` Gerd Hoffmann
2012-10-12 12:39               ` [Qemu-devel] [PATCH v2] " Gerd Hoffmann
2012-10-12 13:12                 ` Gerd Hoffmann
2012-10-12 16:54                 ` Paolo Bonzini
2012-10-12 17:08                   ` Paolo Bonzini
2012-10-15  6:51                 ` Lei Li
2012-10-15 11:09                   ` Andreas Färber
2012-10-16 13:23                     ` Luiz Capitulino
2012-10-15 17:36                   ` Eric Blake
2012-10-16 14:02                     ` Paolo Bonzini
2012-10-12  9:42 ` [Qemu-devel] [PULL 0/7] serial device hotplug patch series Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.