* [Qemu-devel] [PULL 1/3] usb: Remove legacy -usbdevice options (host, serial, disk and net)
2018-01-25 9:16 [Qemu-devel] [PULL 0/3] Usb 20180125 patches Gerd Hoffmann
@ 2018-01-25 9:16 ` Gerd Hoffmann
2018-01-25 9:16 ` [Qemu-devel] [PULL 2/3] hw/usb/ccid: Make ccid_card_init() take an error parameter Gerd Hoffmann
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2018-01-25 9:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Thomas Huth, Gerd Hoffmann, Samuel Thibault
From: Thomas Huth <thuth@redhat.com>
The option have been marked as deprecated since QEMU 2.10, and so far
nobody complained that the host, serial, disk and net options are urgently
required anymore. So let's now get rid at least of this legacy pile, to
simplify the usb code quite a bit.
This patch removes the usbdevices host, serial, disk and net. These devices
use their own complicated parameter parsing mechanisms, so they are just
ugly to maintain, without real benefit for the users (the users can use the
corresponding "-device" parameters instead which have the same complexity
as the "-usbdevice" devices here).
Note that the other rather simple -usbdevice options (mouse, tablet, etc.)
are not removed yet (the code is really simple here, so it does not hurt
much to keep it), as well as the two devices "braille" and "bt" which are
easier to use with -usbdevice than with -device.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-id: 1515519171-20315-1-git-send-email-thuth@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/usb/dev-network.c | 26 ----------
hw/usb/dev-serial.c | 30 -----------
hw/usb/dev-storage.c | 58 ---------------------
hw/usb/host-legacy.c | 144 ---------------------------------------------------
hw/usb/Makefile.objs | 2 +-
qemu-options.hx | 19 -------
6 files changed, 1 insertion(+), 278 deletions(-)
delete mode 100644 hw/usb/host-legacy.c
diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
index 85fc81bf43..aea7edcf31 100644
--- a/hw/usb/dev-network.c
+++ b/hw/usb/dev-network.c
@@ -1382,31 +1382,6 @@ static void usb_net_instance_init(Object *obj)
&dev->qdev, NULL);
}
-static USBDevice *usb_net_init(USBBus *bus, const char *cmdline)
-{
- Error *local_err = NULL;
- USBDevice *dev;
- QemuOpts *opts;
- int idx;
-
- opts = qemu_opts_parse_noisily(qemu_find_opts("net"), cmdline, false);
- if (!opts) {
- return NULL;
- }
- qemu_opt_set(opts, "type", "nic", &error_abort);
- qemu_opt_set(opts, "model", "usb", &error_abort);
-
- idx = net_client_init(opts, false, &local_err);
- if (local_err) {
- error_report_err(local_err);
- return NULL;
- }
-
- dev = usb_create(bus, "usb-net");
- qdev_set_nic_properties(&dev->qdev, &nd_table[idx]);
- return dev;
-}
-
static const VMStateDescription vmstate_usb_net = {
.name = "usb-net",
.unmigratable = 1,
@@ -1446,7 +1421,6 @@ static const TypeInfo net_info = {
static void usb_net_register_types(void)
{
type_register_static(&net_info);
- usb_legacy_register(TYPE_USB_NET, "net", usb_net_init);
}
type_init(usb_net_register_types)
diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index 94b5c34afe..2829dda391 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -509,35 +509,6 @@ static void usb_serial_realize(USBDevice *dev, Error **errp)
}
}
-static USBDevice *usb_serial_init(USBBus *bus, const char *filename)
-{
- USBDevice *dev;
- Chardev *cdrv;
- char label[32];
- static int index;
-
- if (*filename == ':') {
- filename++;
- } else if (*filename) {
- error_report("unrecognized serial USB option %s", filename);
- return NULL;
- }
- if (!*filename) {
- error_report("character device specification needed");
- return NULL;
- }
-
- snprintf(label, sizeof(label), "usbserial%d", index++);
- cdrv = qemu_chr_new(label, filename);
- if (!cdrv)
- return NULL;
-
- dev = usb_create(bus, "usb-serial");
- qdev_prop_set_chr(&dev->qdev, "chardev", cdrv);
-
- return dev;
-}
-
static USBDevice *usb_braille_init(USBBus *bus, const char *unused)
{
USBDevice *dev;
@@ -624,7 +595,6 @@ static void usb_serial_register_types(void)
{
type_register_static(&usb_serial_dev_type_info);
type_register_static(&serial_info);
- usb_legacy_register("usb-serial", "serial", usb_serial_init);
type_register_static(&braille_info);
usb_legacy_register("usb-braille", "braille", usb_braille_init);
}
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 9722ac854c..e44a5c72cf 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -666,63 +666,6 @@ static void usb_msd_bot_realize(USBDevice *dev, Error **errp)
usb_msd_handle_reset(dev);
}
-static USBDevice *usb_msd_init(USBBus *bus, const char *filename)
-{
- static int nr=0;
- Error *err = NULL;
- char id[8];
- QemuOpts *opts;
- DriveInfo *dinfo;
- USBDevice *dev;
- const char *p1;
- char fmt[32];
-
- /* parse -usbdevice disk: syntax into drive opts */
- do {
- snprintf(id, sizeof(id), "usb%d", nr++);
- opts = qemu_opts_create(qemu_find_opts("drive"), id, 1, NULL);
- } while (!opts);
-
- p1 = strchr(filename, ':');
- if (p1++) {
- const char *p2;
-
- if (strstart(filename, "format=", &p2)) {
- int len = MIN(p1 - p2, sizeof(fmt));
- pstrcpy(fmt, len, p2);
- qemu_opt_set(opts, "format", fmt, &error_abort);
- } else if (*filename != ':') {
- error_report("unrecognized USB mass-storage option %s", filename);
- return NULL;
- }
- filename = p1;
- }
- if (!*filename) {
- error_report("block device specification needed");
- return NULL;
- }
- qemu_opt_set(opts, "file", filename, &error_abort);
- qemu_opt_set(opts, "if", "none", &error_abort);
-
- /* create host drive */
- dinfo = drive_new(opts, 0);
- if (!dinfo) {
- qemu_opts_del(opts);
- return NULL;
- }
-
- /* create guest device */
- dev = usb_create(bus, "usb-storage");
- qdev_prop_set_drive(&dev->qdev, "drive", blk_by_legacy_dinfo(dinfo),
- &err);
- if (err) {
- error_report_err(err);
- object_unparent(OBJECT(dev));
- return NULL;
- }
- return dev;
-}
-
static const VMStateDescription vmstate_usb_msd = {
.name = "usb-storage",
.version_id = 1,
@@ -855,7 +798,6 @@ static void usb_msd_register_types(void)
type_register_static(&usb_storage_dev_type_info);
type_register_static(&msd_info);
type_register_static(&bot_info);
- usb_legacy_register("usb-storage", "disk", usb_msd_init);
}
type_init(usb_msd_register_types)
diff --git a/hw/usb/host-legacy.c b/hw/usb/host-legacy.c
deleted file mode 100644
index 3b57e21b52..0000000000
--- a/hw/usb/host-legacy.c
+++ /dev/null
@@ -1,144 +0,0 @@
-/*
- * Linux host USB redirector
- *
- * Copyright (c) 2005 Fabrice Bellard
- *
- * Copyright (c) 2008 Max Krasnyansky
- * Support for host device auto connect & disconnect
- * Major rewrite to support fully async operation
- *
- * Copyright 2008 TJ <linux@tjworld.net>
- * Added flexible support for /dev/bus/usb /sys/bus/usb/devices in addition
- * to the legacy /proc/bus/usb USB device discovery and handling
- *
- * 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 "qemu/osdep.h"
-#include "qemu-common.h"
-#include "hw/usb.h"
-#include "hw/usb/host.h"
-
-/*
- * Autoconnect filter
- * Format:
- * auto:bus:dev[:vid:pid]
- * auto:bus.dev[:vid:pid]
- *
- * bus - bus number (dec, * means any)
- * dev - device number (dec, * means any)
- * vid - vendor id (hex, * means any)
- * pid - product id (hex, * means any)
- *
- * See 'lsusb' output.
- */
-static int parse_filter(const char *spec, struct USBAutoFilter *f)
-{
- enum { BUS, DEV, VID, PID, DONE };
- const char *p = spec;
- int i;
-
- f->bus_num = 0;
- f->addr = 0;
- f->vendor_id = 0;
- f->product_id = 0;
-
- for (i = BUS; i < DONE; i++) {
- p = strpbrk(p, ":.");
- if (!p) {
- break;
- }
- p++;
-
- if (*p == '*') {
- continue;
- }
- switch (i) {
- case BUS:
- f->bus_num = strtol(p, NULL, 10);
- break;
- case DEV:
- f->addr = strtol(p, NULL, 10);
- break;
- case VID:
- f->vendor_id = strtol(p, NULL, 16);
- break;
- case PID:
- f->product_id = strtol(p, NULL, 16);
- break;
- }
- }
-
- if (i < DEV) {
- fprintf(stderr, "husb: invalid auto filter spec %s\n", spec);
- return -1;
- }
-
- return 0;
-}
-
-USBDevice *usb_host_device_open(USBBus *bus, const char *devname)
-{
- struct USBAutoFilter filter;
- USBDevice *dev;
- char *p;
-
- dev = usb_create(bus, "usb-host");
-
- if (strstr(devname, "auto:")) {
- if (parse_filter(devname, &filter) < 0) {
- goto fail;
- }
- } else {
- p = strchr(devname, '.');
- if (p) {
- filter.bus_num = strtoul(devname, NULL, 0);
- filter.addr = strtoul(p + 1, NULL, 0);
- filter.vendor_id = 0;
- filter.product_id = 0;
- } else {
- p = strchr(devname, ':');
- if (p) {
- filter.bus_num = 0;
- filter.addr = 0;
- filter.vendor_id = strtoul(devname, NULL, 16);
- filter.product_id = strtoul(p + 1, NULL, 16);
- } else {
- goto fail;
- }
- }
- }
-
- qdev_prop_set_uint32(&dev->qdev, "hostbus", filter.bus_num);
- qdev_prop_set_uint32(&dev->qdev, "hostaddr", filter.addr);
- qdev_prop_set_uint32(&dev->qdev, "vendorid", filter.vendor_id);
- qdev_prop_set_uint32(&dev->qdev, "productid", filter.product_id);
- return dev;
-
-fail:
- object_unparent(OBJECT(dev));
- return NULL;
-}
-
-static void usb_host_register_types(void)
-{
- usb_legacy_register("usb-host", "host", usb_host_device_open);
-}
-
-type_init(usb_host_register_types)
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index bdfead6701..fbcd498c59 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -43,7 +43,7 @@ redirect.o-libs = $(USB_REDIR_LIBS)
# usb pass-through
ifeq ($(CONFIG_USB_LIBUSB)$(CONFIG_USB),yy)
-common-obj-y += host-libusb.o host-legacy.o
+common-obj-y += host-libusb.o
else
common-obj-y += host-stub.o
endif
diff --git a/qemu-options.hx b/qemu-options.hx
index 5ff741a4af..1d73fb151d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1221,29 +1221,10 @@ Pointer device that uses absolute coordinates (like a touchscreen). This
means QEMU is able to report the mouse position without having to grab the
mouse. Also overrides the PS/2 mouse emulation when activated.
-@item disk:[format=@var{format}]:@var{file}
-Mass storage device based on file. The optional @var{format} argument
-will be used rather than detecting the format. Can be used to specify
-@code{format=raw} to avoid interpreting an untrusted format header.
-
-@item host:@var{bus}.@var{addr}
-Pass through the host device identified by @var{bus}.@var{addr} (Linux only).
-
-@item host:@var{vendor_id}:@var{product_id}
-Pass through the host device identified by @var{vendor_id}:@var{product_id}
-(Linux only).
-
-@item serial:[vendorid=@var{vendor_id}][,productid=@var{product_id}]:@var{dev}
-Serial converter to host character device @var{dev}, see @code{-serial} for the
-available devices.
-
@item braille
Braille device. This will use BrlAPI to display the braille output on a real
or fake device.
-@item net:@var{options}
-Network adapter that supports CDC ethernet and RNDIS protocols.
-
@end table
ETEXI
--
2.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PULL 2/3] hw/usb/ccid: Make ccid_card_init() take an error parameter
2018-01-25 9:16 [Qemu-devel] [PULL 0/3] Usb 20180125 patches Gerd Hoffmann
2018-01-25 9:16 ` [Qemu-devel] [PULL 1/3] usb: Remove legacy -usbdevice options (host, serial, disk and net) Gerd Hoffmann
@ 2018-01-25 9:16 ` Gerd Hoffmann
2018-01-25 16:45 ` Philippe Mathieu-Daudé
2018-01-25 9:16 ` [Qemu-devel] [PULL 3/3] usb-storage: Fix share-rw option parsing Gerd Hoffmann
2018-01-25 15:28 ` [Qemu-devel] [PULL 0/3] Usb 20180125 patches Peter Maydell
3 siblings, 1 reply; 9+ messages in thread
From: Gerd Hoffmann @ 2018-01-25 9:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Mao Zhongyi, Gerd Hoffmann, Marc-André Lureau, Cao jin
From: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Replace init() of CCIDCardClass with realize, then convert
ccid_card_init(), ccid_card_initfn() and it's callbacks to
take an Error** in ordor to report the error more clearly.
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@gmail.com>
Cc: Cao jin <caoj.fnst@cn.fujitsu.com>
Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Message-id: a8b7c670cb61b8096291f5730af62f4230a3e7fa.1514187411.git.maozy.fnst@cn.fujitsu.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/usb/ccid.h | 2 +-
hw/usb/ccid-card-emulated.c | 44 +++++++++++++++++++++----------------------
hw/usb/ccid-card-passthru.c | 12 ++++++------
hw/usb/dev-smartcard-reader.c | 34 ++++++++++++++++++---------------
4 files changed, 48 insertions(+), 44 deletions(-)
diff --git a/hw/usb/ccid.h b/hw/usb/ccid.h
index 1f070116d6..6c6c10188d 100644
--- a/hw/usb/ccid.h
+++ b/hw/usb/ccid.h
@@ -34,7 +34,7 @@ typedef struct CCIDCardClass {
const uint8_t *apdu,
uint32_t len);
void (*exitfn)(CCIDCardState *card);
- int (*initfn)(CCIDCardState *card);
+ void (*realize)(CCIDCardState *card, Error **errp);
} CCIDCardClass;
/*
diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
index e646eb243b..daefd9f8f4 100644
--- a/hw/usb/ccid-card-emulated.c
+++ b/hw/usb/ccid-card-emulated.c
@@ -35,6 +35,7 @@
#include "qemu/thread.h"
#include "qemu/main-loop.h"
#include "ccid.h"
+#include "qapi/error.h"
#define DPRINTF(card, lvl, fmt, ...) \
do {\
@@ -401,10 +402,10 @@ static void card_event_handler(EventNotifier *notifier)
qemu_mutex_unlock(&card->event_list_mutex);
}
-static int init_event_notifier(EmulatedState *card)
+static int init_event_notifier(EmulatedState *card, Error **errp)
{
if (event_notifier_init(&card->notifier, false) < 0) {
- DPRINTF(card, 2, "event notifier creation failed\n");
+ error_setg(errp, "ccid-card-emul: event notifier creation failed");
return -1;
}
event_notifier_set_handler(&card->notifier, card_event_handler);
@@ -480,7 +481,7 @@ static uint32_t parse_enumeration(char *str,
return ret;
}
-static int emulated_initfn(CCIDCardState *base)
+static void emulated_realize(CCIDCardState *base, Error **errp)
{
EmulatedState *card = EMULATED_CCID_CARD(base);
VCardEmulError ret;
@@ -494,8 +495,8 @@ static int emulated_initfn(CCIDCardState *base)
qemu_cond_init(&card->handle_apdu_cond);
card->reader = NULL;
card->quit_apdu_thread = 0;
- if (init_event_notifier(card) < 0) {
- return -1;
+ if (init_event_notifier(card, errp) < 0) {
+ return;
}
card->backend = 0;
@@ -505,11 +506,11 @@ static int emulated_initfn(CCIDCardState *base)
}
if (card->backend == 0) {
- printf("backend must be one of:\n");
+ error_setg(errp, "backend must be one of:");
for (ptable = backend_enum_table; ptable->name != NULL; ++ptable) {
- printf("%s\n", ptable->name);
+ error_append_hint(errp, "%s\n", ptable->name);
}
- return -1;
+ return;
}
/* TODO: a passthru backened that works on local machine. third card type?*/
@@ -517,34 +518,33 @@ static int emulated_initfn(CCIDCardState *base)
if (card->cert1 != NULL && card->cert2 != NULL && card->cert3 != NULL) {
ret = emulated_initialize_vcard_from_certificates(card);
} else {
- printf("%s: you must provide all three certs for"
- " certificates backend\n", TYPE_EMULATED_CCID);
- return -1;
+ error_setg(errp, "%s: you must provide all three certs for"
+ " certificates backend", TYPE_EMULATED_CCID);
+ return;
}
} else {
if (card->backend != BACKEND_NSS_EMULATED) {
- printf("%s: bad backend specified. The options are:\n%s (default),"
- " %s.\n", TYPE_EMULATED_CCID, BACKEND_NSS_EMULATED_NAME,
- BACKEND_CERTIFICATES_NAME);
- return -1;
+ error_setg(errp, "%s: bad backend specified. The options are:%s"
+ " (default), %s.", TYPE_EMULATED_CCID,
+ BACKEND_NSS_EMULATED_NAME, BACKEND_CERTIFICATES_NAME);
+ return;
}
if (card->cert1 != NULL || card->cert2 != NULL || card->cert3 != NULL) {
- printf("%s: unexpected cert parameters to nss emulated backend\n",
- TYPE_EMULATED_CCID);
- return -1;
+ error_setg(errp, "%s: unexpected cert parameters to nss emulated "
+ "backend", TYPE_EMULATED_CCID);
+ return;
}
/* default to mirroring the local hardware readers */
ret = wrap_vcard_emul_init(NULL);
}
if (ret != VCARD_EMUL_OK) {
- printf("%s: failed to initialize vcard\n", TYPE_EMULATED_CCID);
- return -1;
+ error_setg(errp, "%s: failed to initialize vcard", TYPE_EMULATED_CCID);
+ return;
}
qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
card, QEMU_THREAD_JOINABLE);
qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", handle_apdu_thread,
card, QEMU_THREAD_JOINABLE);
- return 0;
}
static void emulated_exitfn(CCIDCardState *base)
@@ -581,7 +581,7 @@ static void emulated_class_initfn(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
CCIDCardClass *cc = CCID_CARD_CLASS(klass);
- cc->initfn = emulated_initfn;
+ cc->realize = emulated_realize;
cc->exitfn = emulated_exitfn;
cc->get_atr = emulated_get_atr;
cc->apdu_from_guest = emulated_apdu_from_guest;
diff --git a/hw/usb/ccid-card-passthru.c b/hw/usb/ccid-card-passthru.c
index 117711862e..b7dd3602dc 100644
--- a/hw/usb/ccid-card-passthru.c
+++ b/hw/usb/ccid-card-passthru.c
@@ -14,6 +14,7 @@
#include "qemu/error-report.h"
#include "qemu/sockets.h"
#include "ccid.h"
+#include "qapi/error.h"
#define DPRINTF(card, lvl, fmt, ...) \
do { \
@@ -337,29 +338,28 @@ static const uint8_t *passthru_get_atr(CCIDCardState *base, uint32_t *len)
return card->atr;
}
-static int passthru_initfn(CCIDCardState *base)
+static void passthru_realize(CCIDCardState *base, Error **errp)
{
PassthruState *card = PASSTHRU_CCID_CARD(base);
card->vscard_in_pos = 0;
card->vscard_in_hdr = 0;
if (qemu_chr_fe_backend_connected(&card->cs)) {
- DPRINTF(card, D_INFO, "initing chardev\n");
+ error_setg(errp, "ccid-card-passthru: initing chardev");
qemu_chr_fe_set_handlers(&card->cs,
ccid_card_vscard_can_read,
ccid_card_vscard_read,
ccid_card_vscard_event, NULL, card, NULL, true);
ccid_card_vscard_send_init(card);
} else {
- error_report("missing chardev");
- return -1;
+ error_setg(errp, "missing chardev");
+ return;
}
card->debug = parse_debug_env("QEMU_CCID_PASSTHRU_DEBUG", D_VERBOSE,
card->debug);
assert(sizeof(DEFAULT_ATR) <= MAX_ATR_SIZE);
memcpy(card->atr, DEFAULT_ATR, sizeof(DEFAULT_ATR));
card->atr_length = sizeof(DEFAULT_ATR);
- return 0;
}
static VMStateDescription passthru_vmstate = {
@@ -387,7 +387,7 @@ static void passthru_class_initfn(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
CCIDCardClass *cc = CCID_CARD_CLASS(klass);
- cc->initfn = passthru_initfn;
+ cc->realize = passthru_realize;
cc->get_atr = passthru_get_atr;
cc->apdu_from_guest = passthru_apdu_from_guest;
set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index e334d3be11..43c564fe7a 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -510,14 +510,18 @@ static void ccid_card_exitfn(CCIDCardState *card)
}
-static int ccid_card_initfn(CCIDCardState *card)
+static void ccid_card_initfn(CCIDCardState *card, Error **errp)
{
CCIDCardClass *cc = CCID_CARD_GET_CLASS(card);
+ Error *local_err = NULL;
- if (cc->initfn) {
- return cc->initfn(card);
+ if (cc->realize) {
+ cc->realize(card, &local_err);
+ if (local_err != NULL) {
+ error_propagate(errp, local_err);
+ return;
+ }
}
- return 0;
}
static bool ccid_has_pending_answers(USBCCIDState *s)
@@ -1295,27 +1299,27 @@ static int ccid_card_exit(DeviceState *qdev)
return 0;
}
-static int ccid_card_init(DeviceState *qdev)
+static void ccid_card_realize(DeviceState *qdev, Error **errp)
{
CCIDCardState *card = CCID_CARD(qdev);
USBDevice *dev = USB_DEVICE(qdev->parent_bus->parent);
USBCCIDState *s = USB_CCID_DEV(dev);
- int ret = 0;
+ Error *local_err = NULL;
if (card->slot != 0) {
- warn_report("usb-ccid supports one slot, can't add %d",
- card->slot);
- return -1;
+ error_setg(errp, "usb-ccid supports one slot, can't add %d",
+ card->slot);
+ return;
}
if (s->card != NULL) {
- warn_report("usb-ccid card already full, not adding");
- return -1;
+ error_setg(errp, "usb-ccid card already full, not adding");
+ return;
}
- ret = ccid_card_initfn(card);
- if (ret == 0) {
+ ccid_card_initfn(card, &local_err);
+ if (local_err != NULL) {
+ error_propagate(errp, local_err);
s->card = card;
}
- return ret;
}
static void ccid_realize(USBDevice *dev, Error **errp)
@@ -1477,7 +1481,7 @@ static void ccid_card_class_init(ObjectClass *klass, void *data)
{
DeviceClass *k = DEVICE_CLASS(klass);
k->bus_type = TYPE_CCID_BUS;
- k->init = ccid_card_init;
+ k->realize = ccid_card_realize;
k->exit = ccid_card_exit;
k->props = ccid_props;
}
--
2.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PULL 2/3] hw/usb/ccid: Make ccid_card_init() take an error parameter
2018-01-25 9:16 ` [Qemu-devel] [PULL 2/3] hw/usb/ccid: Make ccid_card_init() take an error parameter Gerd Hoffmann
@ 2018-01-25 16:45 ` Philippe Mathieu-Daudé
2018-01-25 16:50 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-25 16:45 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: qemu-devel@nongnu.org Developers, Mao Zhongyi,
Marc-André Lureau, Cao jin
Hi Gerd,
On Thu, Jan 25, 2018 at 6:16 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> From: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
>
> Replace init() of CCIDCardClass with realize, then convert
> ccid_card_init(), ccid_card_initfn() and it's callbacks to
> take an Error** in ordor to report the error more clearly.
>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Marc-André Lureau <marcandre.lureau@gmail.com>
> Cc: Cao jin <caoj.fnst@cn.fujitsu.com>
>
> Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
> Message-id: a8b7c670cb61b8096291f5730af62f4230a3e7fa.1514187411.git.maozy.fnst@cn.fujitsu.com
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> hw/usb/ccid.h | 2 +-
> hw/usb/ccid-card-emulated.c | 44 +++++++++++++++++++++----------------------
> hw/usb/ccid-card-passthru.c | 12 ++++++------
> hw/usb/dev-smartcard-reader.c | 34 ++++++++++++++++++---------------
> 4 files changed, 48 insertions(+), 44 deletions(-)
>
> diff --git a/hw/usb/ccid.h b/hw/usb/ccid.h
> index 1f070116d6..6c6c10188d 100644
> --- a/hw/usb/ccid.h
> +++ b/hw/usb/ccid.h
> @@ -34,7 +34,7 @@ typedef struct CCIDCardClass {
> const uint8_t *apdu,
> uint32_t len);
> void (*exitfn)(CCIDCardState *card);
> - int (*initfn)(CCIDCardState *card);
> + void (*realize)(CCIDCardState *card, Error **errp);
> } CCIDCardClass;
>
> /*
> diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
> index e646eb243b..daefd9f8f4 100644
> --- a/hw/usb/ccid-card-emulated.c
> +++ b/hw/usb/ccid-card-emulated.c
> @@ -35,6 +35,7 @@
> #include "qemu/thread.h"
> #include "qemu/main-loop.h"
> #include "ccid.h"
> +#include "qapi/error.h"
>
> #define DPRINTF(card, lvl, fmt, ...) \
> do {\
> @@ -401,10 +402,10 @@ static void card_event_handler(EventNotifier *notifier)
> qemu_mutex_unlock(&card->event_list_mutex);
> }
>
> -static int init_event_notifier(EmulatedState *card)
> +static int init_event_notifier(EmulatedState *card, Error **errp)
> {
> if (event_notifier_init(&card->notifier, false) < 0) {
> - DPRINTF(card, 2, "event notifier creation failed\n");
> + error_setg(errp, "ccid-card-emul: event notifier creation failed");
> return -1;
> }
> event_notifier_set_handler(&card->notifier, card_event_handler);
> @@ -480,7 +481,7 @@ static uint32_t parse_enumeration(char *str,
> return ret;
> }
>
> -static int emulated_initfn(CCIDCardState *base)
> +static void emulated_realize(CCIDCardState *base, Error **errp)
> {
> EmulatedState *card = EMULATED_CCID_CARD(base);
> VCardEmulError ret;
> @@ -494,8 +495,8 @@ static int emulated_initfn(CCIDCardState *base)
> qemu_cond_init(&card->handle_apdu_cond);
> card->reader = NULL;
> card->quit_apdu_thread = 0;
> - if (init_event_notifier(card) < 0) {
> - return -1;
> + if (init_event_notifier(card, errp) < 0) {
> + return;
> }
>
> card->backend = 0;
> @@ -505,11 +506,11 @@ static int emulated_initfn(CCIDCardState *base)
> }
>
> if (card->backend == 0) {
> - printf("backend must be one of:\n");
> + error_setg(errp, "backend must be one of:");
> for (ptable = backend_enum_table; ptable->name != NULL; ++ptable) {
> - printf("%s\n", ptable->name);
> + error_append_hint(errp, "%s\n", ptable->name);
> }
> - return -1;
> + return;
> }
>
> /* TODO: a passthru backened that works on local machine. third card type?*/
> @@ -517,34 +518,33 @@ static int emulated_initfn(CCIDCardState *base)
> if (card->cert1 != NULL && card->cert2 != NULL && card->cert3 != NULL) {
> ret = emulated_initialize_vcard_from_certificates(card);
> } else {
> - printf("%s: you must provide all three certs for"
> - " certificates backend\n", TYPE_EMULATED_CCID);
> - return -1;
> + error_setg(errp, "%s: you must provide all three certs for"
> + " certificates backend", TYPE_EMULATED_CCID);
> + return;
> }
> } else {
> if (card->backend != BACKEND_NSS_EMULATED) {
> - printf("%s: bad backend specified. The options are:\n%s (default),"
> - " %s.\n", TYPE_EMULATED_CCID, BACKEND_NSS_EMULATED_NAME,
> - BACKEND_CERTIFICATES_NAME);
> - return -1;
> + error_setg(errp, "%s: bad backend specified. The options are:%s"
> + " (default), %s.", TYPE_EMULATED_CCID,
> + BACKEND_NSS_EMULATED_NAME, BACKEND_CERTIFICATES_NAME);
> + return;
> }
> if (card->cert1 != NULL || card->cert2 != NULL || card->cert3 != NULL) {
> - printf("%s: unexpected cert parameters to nss emulated backend\n",
> - TYPE_EMULATED_CCID);
> - return -1;
> + error_setg(errp, "%s: unexpected cert parameters to nss emulated "
> + "backend", TYPE_EMULATED_CCID);
> + return;
> }
> /* default to mirroring the local hardware readers */
> ret = wrap_vcard_emul_init(NULL);
> }
> if (ret != VCARD_EMUL_OK) {
> - printf("%s: failed to initialize vcard\n", TYPE_EMULATED_CCID);
> - return -1;
> + error_setg(errp, "%s: failed to initialize vcard", TYPE_EMULATED_CCID);
> + return;
> }
> qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
> card, QEMU_THREAD_JOINABLE);
> qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", handle_apdu_thread,
> card, QEMU_THREAD_JOINABLE);
> - return 0;
> }
>
> static void emulated_exitfn(CCIDCardState *base)
> @@ -581,7 +581,7 @@ static void emulated_class_initfn(ObjectClass *klass, void *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
> CCIDCardClass *cc = CCID_CARD_CLASS(klass);
>
> - cc->initfn = emulated_initfn;
> + cc->realize = emulated_realize;
> cc->exitfn = emulated_exitfn;
> cc->get_atr = emulated_get_atr;
> cc->apdu_from_guest = emulated_apdu_from_guest;
> diff --git a/hw/usb/ccid-card-passthru.c b/hw/usb/ccid-card-passthru.c
> index 117711862e..b7dd3602dc 100644
> --- a/hw/usb/ccid-card-passthru.c
> +++ b/hw/usb/ccid-card-passthru.c
> @@ -14,6 +14,7 @@
> #include "qemu/error-report.h"
> #include "qemu/sockets.h"
> #include "ccid.h"
> +#include "qapi/error.h"
>
> #define DPRINTF(card, lvl, fmt, ...) \
> do { \
> @@ -337,29 +338,28 @@ static const uint8_t *passthru_get_atr(CCIDCardState *base, uint32_t *len)
> return card->atr;
> }
>
> -static int passthru_initfn(CCIDCardState *base)
> +static void passthru_realize(CCIDCardState *base, Error **errp)
> {
> PassthruState *card = PASSTHRU_CCID_CARD(base);
>
> card->vscard_in_pos = 0;
> card->vscard_in_hdr = 0;
> if (qemu_chr_fe_backend_connected(&card->cs)) {
> - DPRINTF(card, D_INFO, "initing chardev\n");
> + error_setg(errp, "ccid-card-passthru: initing chardev");
> qemu_chr_fe_set_handlers(&card->cs,
> ccid_card_vscard_can_read,
> ccid_card_vscard_read,
> ccid_card_vscard_event, NULL, card, NULL, true);
> ccid_card_vscard_send_init(card);
> } else {
> - error_report("missing chardev");
> - return -1;
> + error_setg(errp, "missing chardev");
> + return;
> }
> card->debug = parse_debug_env("QEMU_CCID_PASSTHRU_DEBUG", D_VERBOSE,
> card->debug);
> assert(sizeof(DEFAULT_ATR) <= MAX_ATR_SIZE);
> memcpy(card->atr, DEFAULT_ATR, sizeof(DEFAULT_ATR));
> card->atr_length = sizeof(DEFAULT_ATR);
> - return 0;
> }
>
> static VMStateDescription passthru_vmstate = {
> @@ -387,7 +387,7 @@ static void passthru_class_initfn(ObjectClass *klass, void *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
> CCIDCardClass *cc = CCID_CARD_CLASS(klass);
>
> - cc->initfn = passthru_initfn;
> + cc->realize = passthru_realize;
> cc->get_atr = passthru_get_atr;
> cc->apdu_from_guest = passthru_apdu_from_guest;
> set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
> diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
> index e334d3be11..43c564fe7a 100644
> --- a/hw/usb/dev-smartcard-reader.c
> +++ b/hw/usb/dev-smartcard-reader.c
> @@ -510,14 +510,18 @@ static void ccid_card_exitfn(CCIDCardState *card)
>
> }
>
> -static int ccid_card_initfn(CCIDCardState *card)
> +static void ccid_card_initfn(CCIDCardState *card, Error **errp)
> {
> CCIDCardClass *cc = CCID_CARD_GET_CLASS(card);
> + Error *local_err = NULL;
>
> - if (cc->initfn) {
> - return cc->initfn(card);
> + if (cc->realize) {
> + cc->realize(card, &local_err);
> + if (local_err != NULL) {
> + error_propagate(errp, local_err);
> + return;
> + }
> }
> - return 0;
> }
>
> static bool ccid_has_pending_answers(USBCCIDState *s)
> @@ -1295,27 +1299,27 @@ static int ccid_card_exit(DeviceState *qdev)
> return 0;
> }
>
> -static int ccid_card_init(DeviceState *qdev)
> +static void ccid_card_realize(DeviceState *qdev, Error **errp)
> {
> CCIDCardState *card = CCID_CARD(qdev);
> USBDevice *dev = USB_DEVICE(qdev->parent_bus->parent);
> USBCCIDState *s = USB_CCID_DEV(dev);
> - int ret = 0;
> + Error *local_err = NULL;
>
> if (card->slot != 0) {
> - warn_report("usb-ccid supports one slot, can't add %d",
> - card->slot);
> - return -1;
> + error_setg(errp, "usb-ccid supports one slot, can't add %d",
> + card->slot);
> + return;
> }
> if (s->card != NULL) {
> - warn_report("usb-ccid card already full, not adding");
> - return -1;
> + error_setg(errp, "usb-ccid card already full, not adding");
> + return;
> }
> - ret = ccid_card_initfn(card);
> - if (ret == 0) {
> + ccid_card_initfn(card, &local_err);
> + if (local_err != NULL) {
> + error_propagate(errp, local_err);
noticed while rebasing, is this missing?
} else {
> s->card = card;
> }
> - return ret;
> }
>
> static void ccid_realize(USBDevice *dev, Error **errp)
> @@ -1477,7 +1481,7 @@ static void ccid_card_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *k = DEVICE_CLASS(klass);
> k->bus_type = TYPE_CCID_BUS;
> - k->init = ccid_card_init;
> + k->realize = ccid_card_realize;
> k->exit = ccid_card_exit;
> k->props = ccid_props;
> }
> --
> 2.9.3
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PULL 2/3] hw/usb/ccid: Make ccid_card_init() take an error parameter
2018-01-25 16:45 ` Philippe Mathieu-Daudé
@ 2018-01-25 16:50 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-01-25 16:50 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: qemu-devel@nongnu.org Developers, Mao Zhongyi,
Marc-André Lureau, Cao jin
On Thu, Jan 25, 2018 at 1:45 PM, Philippe Mathieu-Daudé
<philippe@mathieu-daude.net> wrote:
> Hi Gerd,
>
> On Thu, Jan 25, 2018 at 6:16 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
>> From: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
>>
>> Replace init() of CCIDCardClass with realize, then convert
>> ccid_card_init(), ccid_card_initfn() and it's callbacks to
>> take an Error** in ordor to report the error more clearly.
>>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Cc: Marc-André Lureau <marcandre.lureau@gmail.com>
>> Cc: Cao jin <caoj.fnst@cn.fujitsu.com>
>>
>> Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
>> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
>> Message-id: a8b7c670cb61b8096291f5730af62f4230a3e7fa.1514187411.git.maozy.fnst@cn.fujitsu.com
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>> hw/usb/ccid.h | 2 +-
>> hw/usb/ccid-card-emulated.c | 44 +++++++++++++++++++++----------------------
>> hw/usb/ccid-card-passthru.c | 12 ++++++------
>> hw/usb/dev-smartcard-reader.c | 34 ++++++++++++++++++---------------
>> 4 files changed, 48 insertions(+), 44 deletions(-)
>>
>> diff --git a/hw/usb/ccid.h b/hw/usb/ccid.h
>> index 1f070116d6..6c6c10188d 100644
>> --- a/hw/usb/ccid.h
>> +++ b/hw/usb/ccid.h
>> @@ -34,7 +34,7 @@ typedef struct CCIDCardClass {
>> const uint8_t *apdu,
>> uint32_t len);
>> void (*exitfn)(CCIDCardState *card);
>> - int (*initfn)(CCIDCardState *card);
>> + void (*realize)(CCIDCardState *card, Error **errp);
>> } CCIDCardClass;
>>
>> /*
>> diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
>> index e646eb243b..daefd9f8f4 100644
>> --- a/hw/usb/ccid-card-emulated.c
>> +++ b/hw/usb/ccid-card-emulated.c
>> @@ -35,6 +35,7 @@
>> #include "qemu/thread.h"
>> #include "qemu/main-loop.h"
>> #include "ccid.h"
>> +#include "qapi/error.h"
>>
>> #define DPRINTF(card, lvl, fmt, ...) \
>> do {\
>> @@ -401,10 +402,10 @@ static void card_event_handler(EventNotifier *notifier)
>> qemu_mutex_unlock(&card->event_list_mutex);
>> }
>>
>> -static int init_event_notifier(EmulatedState *card)
>> +static int init_event_notifier(EmulatedState *card, Error **errp)
>> {
>> if (event_notifier_init(&card->notifier, false) < 0) {
>> - DPRINTF(card, 2, "event notifier creation failed\n");
>> + error_setg(errp, "ccid-card-emul: event notifier creation failed");
>> return -1;
>> }
>> event_notifier_set_handler(&card->notifier, card_event_handler);
>> @@ -480,7 +481,7 @@ static uint32_t parse_enumeration(char *str,
>> return ret;
>> }
>>
>> -static int emulated_initfn(CCIDCardState *base)
>> +static void emulated_realize(CCIDCardState *base, Error **errp)
>> {
>> EmulatedState *card = EMULATED_CCID_CARD(base);
>> VCardEmulError ret;
>> @@ -494,8 +495,8 @@ static int emulated_initfn(CCIDCardState *base)
>> qemu_cond_init(&card->handle_apdu_cond);
>> card->reader = NULL;
>> card->quit_apdu_thread = 0;
>> - if (init_event_notifier(card) < 0) {
>> - return -1;
>> + if (init_event_notifier(card, errp) < 0) {
>> + return;
>> }
>>
>> card->backend = 0;
>> @@ -505,11 +506,11 @@ static int emulated_initfn(CCIDCardState *base)
>> }
>>
>> if (card->backend == 0) {
>> - printf("backend must be one of:\n");
>> + error_setg(errp, "backend must be one of:");
>> for (ptable = backend_enum_table; ptable->name != NULL; ++ptable) {
>> - printf("%s\n", ptable->name);
>> + error_append_hint(errp, "%s\n", ptable->name);
>> }
>> - return -1;
>> + return;
>> }
>>
>> /* TODO: a passthru backened that works on local machine. third card type?*/
>> @@ -517,34 +518,33 @@ static int emulated_initfn(CCIDCardState *base)
>> if (card->cert1 != NULL && card->cert2 != NULL && card->cert3 != NULL) {
>> ret = emulated_initialize_vcard_from_certificates(card);
>> } else {
>> - printf("%s: you must provide all three certs for"
>> - " certificates backend\n", TYPE_EMULATED_CCID);
>> - return -1;
>> + error_setg(errp, "%s: you must provide all three certs for"
>> + " certificates backend", TYPE_EMULATED_CCID);
>> + return;
>> }
>> } else {
>> if (card->backend != BACKEND_NSS_EMULATED) {
>> - printf("%s: bad backend specified. The options are:\n%s (default),"
>> - " %s.\n", TYPE_EMULATED_CCID, BACKEND_NSS_EMULATED_NAME,
>> - BACKEND_CERTIFICATES_NAME);
>> - return -1;
>> + error_setg(errp, "%s: bad backend specified. The options are:%s"
>> + " (default), %s.", TYPE_EMULATED_CCID,
>> + BACKEND_NSS_EMULATED_NAME, BACKEND_CERTIFICATES_NAME);
>> + return;
>> }
>> if (card->cert1 != NULL || card->cert2 != NULL || card->cert3 != NULL) {
>> - printf("%s: unexpected cert parameters to nss emulated backend\n",
>> - TYPE_EMULATED_CCID);
>> - return -1;
>> + error_setg(errp, "%s: unexpected cert parameters to nss emulated "
>> + "backend", TYPE_EMULATED_CCID);
>> + return;
>> }
>> /* default to mirroring the local hardware readers */
>> ret = wrap_vcard_emul_init(NULL);
>> }
>> if (ret != VCARD_EMUL_OK) {
>> - printf("%s: failed to initialize vcard\n", TYPE_EMULATED_CCID);
>> - return -1;
>> + error_setg(errp, "%s: failed to initialize vcard", TYPE_EMULATED_CCID);
>> + return;
>> }
>> qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
>> card, QEMU_THREAD_JOINABLE);
>> qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", handle_apdu_thread,
>> card, QEMU_THREAD_JOINABLE);
>> - return 0;
>> }
>>
>> static void emulated_exitfn(CCIDCardState *base)
>> @@ -581,7 +581,7 @@ static void emulated_class_initfn(ObjectClass *klass, void *data)
>> DeviceClass *dc = DEVICE_CLASS(klass);
>> CCIDCardClass *cc = CCID_CARD_CLASS(klass);
>>
>> - cc->initfn = emulated_initfn;
>> + cc->realize = emulated_realize;
>> cc->exitfn = emulated_exitfn;
>> cc->get_atr = emulated_get_atr;
>> cc->apdu_from_guest = emulated_apdu_from_guest;
>> diff --git a/hw/usb/ccid-card-passthru.c b/hw/usb/ccid-card-passthru.c
>> index 117711862e..b7dd3602dc 100644
>> --- a/hw/usb/ccid-card-passthru.c
>> +++ b/hw/usb/ccid-card-passthru.c
>> @@ -14,6 +14,7 @@
>> #include "qemu/error-report.h"
>> #include "qemu/sockets.h"
>> #include "ccid.h"
>> +#include "qapi/error.h"
>>
>> #define DPRINTF(card, lvl, fmt, ...) \
>> do { \
>> @@ -337,29 +338,28 @@ static const uint8_t *passthru_get_atr(CCIDCardState *base, uint32_t *len)
>> return card->atr;
>> }
>>
>> -static int passthru_initfn(CCIDCardState *base)
>> +static void passthru_realize(CCIDCardState *base, Error **errp)
>> {
>> PassthruState *card = PASSTHRU_CCID_CARD(base);
>>
>> card->vscard_in_pos = 0;
>> card->vscard_in_hdr = 0;
>> if (qemu_chr_fe_backend_connected(&card->cs)) {
>> - DPRINTF(card, D_INFO, "initing chardev\n");
>> + error_setg(errp, "ccid-card-passthru: initing chardev");
>> qemu_chr_fe_set_handlers(&card->cs,
>> ccid_card_vscard_can_read,
>> ccid_card_vscard_read,
>> ccid_card_vscard_event, NULL, card, NULL, true);
>> ccid_card_vscard_send_init(card);
>> } else {
>> - error_report("missing chardev");
>> - return -1;
>> + error_setg(errp, "missing chardev");
>> + return;
>> }
>> card->debug = parse_debug_env("QEMU_CCID_PASSTHRU_DEBUG", D_VERBOSE,
>> card->debug);
>> assert(sizeof(DEFAULT_ATR) <= MAX_ATR_SIZE);
>> memcpy(card->atr, DEFAULT_ATR, sizeof(DEFAULT_ATR));
>> card->atr_length = sizeof(DEFAULT_ATR);
>> - return 0;
>> }
>>
>> static VMStateDescription passthru_vmstate = {
>> @@ -387,7 +387,7 @@ static void passthru_class_initfn(ObjectClass *klass, void *data)
>> DeviceClass *dc = DEVICE_CLASS(klass);
>> CCIDCardClass *cc = CCID_CARD_CLASS(klass);
>>
>> - cc->initfn = passthru_initfn;
>> + cc->realize = passthru_realize;
>> cc->get_atr = passthru_get_atr;
>> cc->apdu_from_guest = passthru_apdu_from_guest;
>> set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
>> diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
>> index e334d3be11..43c564fe7a 100644
>> --- a/hw/usb/dev-smartcard-reader.c
>> +++ b/hw/usb/dev-smartcard-reader.c
>> @@ -510,14 +510,18 @@ static void ccid_card_exitfn(CCIDCardState *card)
>>
>> }
>>
>> -static int ccid_card_initfn(CCIDCardState *card)
>> +static void ccid_card_initfn(CCIDCardState *card, Error **errp)
>> {
>> CCIDCardClass *cc = CCID_CARD_GET_CLASS(card);
>> + Error *local_err = NULL;
>>
>> - if (cc->initfn) {
>> - return cc->initfn(card);
>> + if (cc->realize) {
>> + cc->realize(card, &local_err);
>> + if (local_err != NULL) {
>> + error_propagate(errp, local_err);
>> + return;
>> + }
>> }
>> - return 0;
>> }
>>
>> static bool ccid_has_pending_answers(USBCCIDState *s)
>> @@ -1295,27 +1299,27 @@ static int ccid_card_exit(DeviceState *qdev)
>> return 0;
>> }
>>
>> -static int ccid_card_init(DeviceState *qdev)
>> +static void ccid_card_realize(DeviceState *qdev, Error **errp)
>> {
>> CCIDCardState *card = CCID_CARD(qdev);
>> USBDevice *dev = USB_DEVICE(qdev->parent_bus->parent);
>> USBCCIDState *s = USB_CCID_DEV(dev);
>> - int ret = 0;
>> + Error *local_err = NULL;
>>
>> if (card->slot != 0) {
>> - warn_report("usb-ccid supports one slot, can't add %d",
>> - card->slot);
>> - return -1;
>> + error_setg(errp, "usb-ccid supports one slot, can't add %d",
>> + card->slot);
>> + return;
>> }
>> if (s->card != NULL) {
>> - warn_report("usb-ccid card already full, not adding");
>> - return -1;
>> + error_setg(errp, "usb-ccid card already full, not adding");
>> + return;
>> }
>> - ret = ccid_card_initfn(card);
>> - if (ret == 0) {
>> + ccid_card_initfn(card, &local_err);
>> + if (local_err != NULL) {
>> + error_propagate(errp, local_err);
>
> noticed while rebasing, is this missing?
simpler/cleaner, move out of the if:
+ return;
+ }
+ s->card = card;
- }
>> - return ret;
>> }
>>
>> static void ccid_realize(USBDevice *dev, Error **errp)
>> @@ -1477,7 +1481,7 @@ static void ccid_card_class_init(ObjectClass *klass, void *data)
>> {
>> DeviceClass *k = DEVICE_CLASS(klass);
>> k->bus_type = TYPE_CCID_BUS;
>> - k->init = ccid_card_init;
>> + k->realize = ccid_card_realize;
>> k->exit = ccid_card_exit;
>> k->props = ccid_props;
>> }
>> --
>> 2.9.3
>>
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PULL 3/3] usb-storage: Fix share-rw option parsing
2018-01-25 9:16 [Qemu-devel] [PULL 0/3] Usb 20180125 patches Gerd Hoffmann
2018-01-25 9:16 ` [Qemu-devel] [PULL 1/3] usb: Remove legacy -usbdevice options (host, serial, disk and net) Gerd Hoffmann
2018-01-25 9:16 ` [Qemu-devel] [PULL 2/3] hw/usb/ccid: Make ccid_card_init() take an error parameter Gerd Hoffmann
@ 2018-01-25 9:16 ` Gerd Hoffmann
2018-01-25 15:28 ` [Qemu-devel] [PULL 0/3] Usb 20180125 patches Peter Maydell
3 siblings, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2018-01-25 9:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng, qemu-stable, Gerd Hoffmann, Paolo Bonzini
From: Fam Zheng <famz@redhat.com>
Because usb-storage creates an internal scsi device, we should propagate
options. We already do so for bootindex etc, but failed to take care of
share-rw. Fix it in an apparent way: add a new parameter to
scsi_bus_legacy_add_drive and pass in s->conf.share_rw.
Cc: qemu-stable@nongnu.org
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Message-id: 20180117005222.4781-1-famz@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
include/hw/scsi/scsi.h | 1 +
hw/scsi/scsi-bus.c | 9 ++++++++-
hw/usb/dev-storage.c | 3 ++-
3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 23a8ee6a7d..802a647cdc 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -151,6 +151,7 @@ static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d)
SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk,
int unit, bool removable, int bootindex,
+ bool share_rw,
const char *serial, Error **errp);
void scsi_bus_legacy_handle_cmdline(SCSIBus *bus, bool deprecated);
void scsi_legacy_handle_cmdline(void);
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 965becf31f..05e501efd3 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -224,6 +224,7 @@ static void scsi_qdev_unrealize(DeviceState *qdev, Error **errp)
/* handle legacy '-drive if=scsi,...' cmd line args */
SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk,
int unit, bool removable, int bootindex,
+ bool share_rw,
const char *serial, Error **errp)
{
const char *driver;
@@ -254,6 +255,12 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, BlockBackend *blk,
object_unparent(OBJECT(dev));
return NULL;
}
+ object_property_set_bool(OBJECT(dev), share_rw, "share-rw", &err);
+ if (err != NULL) {
+ error_propagate(errp, err);
+ object_unparent(OBJECT(dev));
+ return NULL;
+ }
object_property_set_bool(OBJECT(dev), true, "realized", &err);
if (err != NULL) {
error_propagate(errp, err);
@@ -288,7 +295,7 @@ void scsi_bus_legacy_handle_cmdline(SCSIBus *bus, bool deprecated)
}
}
scsi_bus_legacy_add_drive(bus, blk_by_legacy_dinfo(dinfo),
- unit, false, -1, NULL, &error_fatal);
+ unit, false, -1, false, NULL, &error_fatal);
}
loc_pop(&loc);
}
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index e44a5c72cf..b56c75a73a 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -633,7 +633,8 @@ static void usb_msd_storage_realize(USBDevice *dev, Error **errp)
scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(dev),
&usb_msd_scsi_info_storage, NULL);
scsi_dev = scsi_bus_legacy_add_drive(&s->bus, blk, 0, !!s->removable,
- s->conf.bootindex, dev->serial,
+ s->conf.bootindex, s->conf.share_rw,
+ dev->serial,
errp);
blk_unref(blk);
if (!scsi_dev) {
--
2.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PULL 0/3] Usb 20180125 patches
2018-01-25 9:16 [Qemu-devel] [PULL 0/3] Usb 20180125 patches Gerd Hoffmann
` (2 preceding siblings ...)
2018-01-25 9:16 ` [Qemu-devel] [PULL 3/3] usb-storage: Fix share-rw option parsing Gerd Hoffmann
@ 2018-01-25 15:28 ` Peter Maydell
2018-01-25 16:24 ` Gerd Hoffmann
3 siblings, 1 reply; 9+ messages in thread
From: Peter Maydell @ 2018-01-25 15:28 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: QEMU Developers
On 25 January 2018 at 09:16, Gerd Hoffmann <kraxel@redhat.com> wrote:
> The following changes since commit 834a336eb911db8a8ca00e760ee6a85faca19414:
>
> Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2018-01-24 19:24:26 +0000)
>
> are available in the git repository at:
>
> git://git.kraxel.org/qemu tags/usb-20180125-pull-request
>
> for you to fetch changes up to 2aef004483e34a42021dc98a3646971dc152cb1b:
>
> usb-storage: Fix share-rw option parsing (2018-01-25 09:50:30 +0100)
>
> ----------------------------------------------------------------
> usb: -usbdevice cleanups, storage fix, ccid cleanup.
>
> ----------------------------------------------------------------
>
> Fam Zheng (1):
> usb-storage: Fix share-rw option parsing
>
> Mao Zhongyi (1):
> hw/usb/ccid: Make ccid_card_init() take an error parameter
>
> Thomas Huth (1):
> usb: Remove legacy -usbdevice options (host, serial, disk and net)
Hi. This fails to build on FreeBSD:
LINK arm-softmmu/qemu-system-arm
../vl.o: In function `usb_device_add':
/root/qemu/vl.c:1471: undefined reference to `usb_host_device_open'
There's still a call to that function in vl.c inside an
ifndef CONFIG_LINUX...
If we can really remove the function we should get rid of the
stub in hw/usb/host-stub.c and the prototype in usb.h too.
thanks
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PULL 0/3] Usb 20180125 patches
2018-01-25 15:28 ` [Qemu-devel] [PULL 0/3] Usb 20180125 patches Peter Maydell
@ 2018-01-25 16:24 ` Gerd Hoffmann
0 siblings, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2018-01-25 16:24 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers
On Thu, Jan 25, 2018 at 03:28:38PM +0000, Peter Maydell wrote:
> On 25 January 2018 at 09:16, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > Thomas Huth (1):
> > usb: Remove legacy -usbdevice options (host, serial, disk and net)
>
> Hi. This fails to build on FreeBSD:
>
> LINK arm-softmmu/qemu-system-arm
> ../vl.o: In function `usb_device_add':
> /root/qemu/vl.c:1471: undefined reference to `usb_host_device_open'
>
> There's still a call to that function in vl.c inside an
> ifndef CONFIG_LINUX...
>
> If we can really remove the function we should get rid of the
> stub in hw/usb/host-stub.c and the prototype in usb.h too.
Indeed. I'll fix it up.
The lines in vl.c dead code anyway since we switched to libusb and
dropped the linux/bsd specific usb-host code, so it all can simply be
deleted.
cheers,
Gerd
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PULL 2/3] hw/usb/ccid: Make ccid_card_init() take an error parameter
2018-01-25 16:43 [Qemu-devel] [PULL 0/3] Usb 20180125 v2 patches Gerd Hoffmann
@ 2018-01-25 16:43 ` Gerd Hoffmann
0 siblings, 0 replies; 9+ messages in thread
From: Gerd Hoffmann @ 2018-01-25 16:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Mao Zhongyi, Gerd Hoffmann, Marc-André Lureau, Cao jin
From: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Replace init() of CCIDCardClass with realize, then convert
ccid_card_init(), ccid_card_initfn() and it's callbacks to
take an Error** in ordor to report the error more clearly.
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@gmail.com>
Cc: Cao jin <caoj.fnst@cn.fujitsu.com>
Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Message-id: a8b7c670cb61b8096291f5730af62f4230a3e7fa.1514187411.git.maozy.fnst@cn.fujitsu.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/usb/ccid.h | 2 +-
hw/usb/ccid-card-emulated.c | 44 +++++++++++++++++++++----------------------
hw/usb/ccid-card-passthru.c | 12 ++++++------
hw/usb/dev-smartcard-reader.c | 34 ++++++++++++++++++---------------
4 files changed, 48 insertions(+), 44 deletions(-)
diff --git a/hw/usb/ccid.h b/hw/usb/ccid.h
index 1f070116d6..6c6c10188d 100644
--- a/hw/usb/ccid.h
+++ b/hw/usb/ccid.h
@@ -34,7 +34,7 @@ typedef struct CCIDCardClass {
const uint8_t *apdu,
uint32_t len);
void (*exitfn)(CCIDCardState *card);
- int (*initfn)(CCIDCardState *card);
+ void (*realize)(CCIDCardState *card, Error **errp);
} CCIDCardClass;
/*
diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
index e646eb243b..daefd9f8f4 100644
--- a/hw/usb/ccid-card-emulated.c
+++ b/hw/usb/ccid-card-emulated.c
@@ -35,6 +35,7 @@
#include "qemu/thread.h"
#include "qemu/main-loop.h"
#include "ccid.h"
+#include "qapi/error.h"
#define DPRINTF(card, lvl, fmt, ...) \
do {\
@@ -401,10 +402,10 @@ static void card_event_handler(EventNotifier *notifier)
qemu_mutex_unlock(&card->event_list_mutex);
}
-static int init_event_notifier(EmulatedState *card)
+static int init_event_notifier(EmulatedState *card, Error **errp)
{
if (event_notifier_init(&card->notifier, false) < 0) {
- DPRINTF(card, 2, "event notifier creation failed\n");
+ error_setg(errp, "ccid-card-emul: event notifier creation failed");
return -1;
}
event_notifier_set_handler(&card->notifier, card_event_handler);
@@ -480,7 +481,7 @@ static uint32_t parse_enumeration(char *str,
return ret;
}
-static int emulated_initfn(CCIDCardState *base)
+static void emulated_realize(CCIDCardState *base, Error **errp)
{
EmulatedState *card = EMULATED_CCID_CARD(base);
VCardEmulError ret;
@@ -494,8 +495,8 @@ static int emulated_initfn(CCIDCardState *base)
qemu_cond_init(&card->handle_apdu_cond);
card->reader = NULL;
card->quit_apdu_thread = 0;
- if (init_event_notifier(card) < 0) {
- return -1;
+ if (init_event_notifier(card, errp) < 0) {
+ return;
}
card->backend = 0;
@@ -505,11 +506,11 @@ static int emulated_initfn(CCIDCardState *base)
}
if (card->backend == 0) {
- printf("backend must be one of:\n");
+ error_setg(errp, "backend must be one of:");
for (ptable = backend_enum_table; ptable->name != NULL; ++ptable) {
- printf("%s\n", ptable->name);
+ error_append_hint(errp, "%s\n", ptable->name);
}
- return -1;
+ return;
}
/* TODO: a passthru backened that works on local machine. third card type?*/
@@ -517,34 +518,33 @@ static int emulated_initfn(CCIDCardState *base)
if (card->cert1 != NULL && card->cert2 != NULL && card->cert3 != NULL) {
ret = emulated_initialize_vcard_from_certificates(card);
} else {
- printf("%s: you must provide all three certs for"
- " certificates backend\n", TYPE_EMULATED_CCID);
- return -1;
+ error_setg(errp, "%s: you must provide all three certs for"
+ " certificates backend", TYPE_EMULATED_CCID);
+ return;
}
} else {
if (card->backend != BACKEND_NSS_EMULATED) {
- printf("%s: bad backend specified. The options are:\n%s (default),"
- " %s.\n", TYPE_EMULATED_CCID, BACKEND_NSS_EMULATED_NAME,
- BACKEND_CERTIFICATES_NAME);
- return -1;
+ error_setg(errp, "%s: bad backend specified. The options are:%s"
+ " (default), %s.", TYPE_EMULATED_CCID,
+ BACKEND_NSS_EMULATED_NAME, BACKEND_CERTIFICATES_NAME);
+ return;
}
if (card->cert1 != NULL || card->cert2 != NULL || card->cert3 != NULL) {
- printf("%s: unexpected cert parameters to nss emulated backend\n",
- TYPE_EMULATED_CCID);
- return -1;
+ error_setg(errp, "%s: unexpected cert parameters to nss emulated "
+ "backend", TYPE_EMULATED_CCID);
+ return;
}
/* default to mirroring the local hardware readers */
ret = wrap_vcard_emul_init(NULL);
}
if (ret != VCARD_EMUL_OK) {
- printf("%s: failed to initialize vcard\n", TYPE_EMULATED_CCID);
- return -1;
+ error_setg(errp, "%s: failed to initialize vcard", TYPE_EMULATED_CCID);
+ return;
}
qemu_thread_create(&card->event_thread_id, "ccid/event", event_thread,
card, QEMU_THREAD_JOINABLE);
qemu_thread_create(&card->apdu_thread_id, "ccid/apdu", handle_apdu_thread,
card, QEMU_THREAD_JOINABLE);
- return 0;
}
static void emulated_exitfn(CCIDCardState *base)
@@ -581,7 +581,7 @@ static void emulated_class_initfn(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
CCIDCardClass *cc = CCID_CARD_CLASS(klass);
- cc->initfn = emulated_initfn;
+ cc->realize = emulated_realize;
cc->exitfn = emulated_exitfn;
cc->get_atr = emulated_get_atr;
cc->apdu_from_guest = emulated_apdu_from_guest;
diff --git a/hw/usb/ccid-card-passthru.c b/hw/usb/ccid-card-passthru.c
index 117711862e..b7dd3602dc 100644
--- a/hw/usb/ccid-card-passthru.c
+++ b/hw/usb/ccid-card-passthru.c
@@ -14,6 +14,7 @@
#include "qemu/error-report.h"
#include "qemu/sockets.h"
#include "ccid.h"
+#include "qapi/error.h"
#define DPRINTF(card, lvl, fmt, ...) \
do { \
@@ -337,29 +338,28 @@ static const uint8_t *passthru_get_atr(CCIDCardState *base, uint32_t *len)
return card->atr;
}
-static int passthru_initfn(CCIDCardState *base)
+static void passthru_realize(CCIDCardState *base, Error **errp)
{
PassthruState *card = PASSTHRU_CCID_CARD(base);
card->vscard_in_pos = 0;
card->vscard_in_hdr = 0;
if (qemu_chr_fe_backend_connected(&card->cs)) {
- DPRINTF(card, D_INFO, "initing chardev\n");
+ error_setg(errp, "ccid-card-passthru: initing chardev");
qemu_chr_fe_set_handlers(&card->cs,
ccid_card_vscard_can_read,
ccid_card_vscard_read,
ccid_card_vscard_event, NULL, card, NULL, true);
ccid_card_vscard_send_init(card);
} else {
- error_report("missing chardev");
- return -1;
+ error_setg(errp, "missing chardev");
+ return;
}
card->debug = parse_debug_env("QEMU_CCID_PASSTHRU_DEBUG", D_VERBOSE,
card->debug);
assert(sizeof(DEFAULT_ATR) <= MAX_ATR_SIZE);
memcpy(card->atr, DEFAULT_ATR, sizeof(DEFAULT_ATR));
card->atr_length = sizeof(DEFAULT_ATR);
- return 0;
}
static VMStateDescription passthru_vmstate = {
@@ -387,7 +387,7 @@ static void passthru_class_initfn(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
CCIDCardClass *cc = CCID_CARD_CLASS(klass);
- cc->initfn = passthru_initfn;
+ cc->realize = passthru_realize;
cc->get_atr = passthru_get_atr;
cc->apdu_from_guest = passthru_apdu_from_guest;
set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index e334d3be11..43c564fe7a 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -510,14 +510,18 @@ static void ccid_card_exitfn(CCIDCardState *card)
}
-static int ccid_card_initfn(CCIDCardState *card)
+static void ccid_card_initfn(CCIDCardState *card, Error **errp)
{
CCIDCardClass *cc = CCID_CARD_GET_CLASS(card);
+ Error *local_err = NULL;
- if (cc->initfn) {
- return cc->initfn(card);
+ if (cc->realize) {
+ cc->realize(card, &local_err);
+ if (local_err != NULL) {
+ error_propagate(errp, local_err);
+ return;
+ }
}
- return 0;
}
static bool ccid_has_pending_answers(USBCCIDState *s)
@@ -1295,27 +1299,27 @@ static int ccid_card_exit(DeviceState *qdev)
return 0;
}
-static int ccid_card_init(DeviceState *qdev)
+static void ccid_card_realize(DeviceState *qdev, Error **errp)
{
CCIDCardState *card = CCID_CARD(qdev);
USBDevice *dev = USB_DEVICE(qdev->parent_bus->parent);
USBCCIDState *s = USB_CCID_DEV(dev);
- int ret = 0;
+ Error *local_err = NULL;
if (card->slot != 0) {
- warn_report("usb-ccid supports one slot, can't add %d",
- card->slot);
- return -1;
+ error_setg(errp, "usb-ccid supports one slot, can't add %d",
+ card->slot);
+ return;
}
if (s->card != NULL) {
- warn_report("usb-ccid card already full, not adding");
- return -1;
+ error_setg(errp, "usb-ccid card already full, not adding");
+ return;
}
- ret = ccid_card_initfn(card);
- if (ret == 0) {
+ ccid_card_initfn(card, &local_err);
+ if (local_err != NULL) {
+ error_propagate(errp, local_err);
s->card = card;
}
- return ret;
}
static void ccid_realize(USBDevice *dev, Error **errp)
@@ -1477,7 +1481,7 @@ static void ccid_card_class_init(ObjectClass *klass, void *data)
{
DeviceClass *k = DEVICE_CLASS(klass);
k->bus_type = TYPE_CCID_BUS;
- k->init = ccid_card_init;
+ k->realize = ccid_card_realize;
k->exit = ccid_card_exit;
k->props = ccid_props;
}
--
2.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread