From patchwork Thu Aug 13 11:05:45 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gerd Hoffmann X-Patchwork-Id: 31291 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by bilbo.ozlabs.org (Postfix) with ESMTPS id D9D11B6EDF for ; Thu, 13 Aug 2009 21:08:42 +1000 (EST) Received: from localhost ([127.0.0.1]:37475 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MbYAg-0004NC-DC for incoming@patchwork.ozlabs.org; Thu, 13 Aug 2009 07:08:38 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MbYA3-0004LA-BT for qemu-devel@nongnu.org; Thu, 13 Aug 2009 07:07:59 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MbY9x-0004Hj-GQ for qemu-devel@nongnu.org; Thu, 13 Aug 2009 07:07:57 -0400 Received: from [199.232.76.173] (port=57317 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MbY9x-0004Hg-DD for qemu-devel@nongnu.org; Thu, 13 Aug 2009 07:07:53 -0400 Received: from mx2.redhat.com ([66.187.237.31]:45170) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MbY9w-0005mg-Li for qemu-devel@nongnu.org; Thu, 13 Aug 2009 07:07:53 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n7DB5pbr001677; Thu, 13 Aug 2009 07:05:51 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n7DB5oSi001648; Thu, 13 Aug 2009 07:05:50 -0400 Received: from zweiblum.home.kraxel.org (vpn2-8-91.ams2.redhat.com [10.36.8.91]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n7DB5kPB013462; Thu, 13 Aug 2009 07:05:47 -0400 Message-ID: <4A83F389.7080302@redhat.com> Date: Thu, 13 Aug 2009 13:05:45 +0200 From: Gerd Hoffmann User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b3pre) Gecko/20090513 Fedora/3.0-2.3.beta2.fc11 Lightning/1.0pre Thunderbird/3.0b2 MIME-Version: 1.0 To: Markus Armbruster Subject: Re: [Qemu-devel] [PATCH] qdev: add return value to init() callbacks. References: <1250092766-23986-1-git-send-email-kraxel@redhat.com> <200908122028.41012.paul@codesourcery.com> <4A831C47.3070309@redhat.com> <873a7w2q6n.fsf@pike.pond.sub.org> <4A83C562.1090802@redhat.com> <87tz0cytem.fsf@pike.pond.sub.org> In-Reply-To: <87tz0cytem.fsf@pike.pond.sub.org> X-Scanned-By: MIMEDefang 2.58 on 172.16.27.26 X-detected-operating-system: by monty-python.gnu.org: GNU/Linux 2.6 (newer, 3) Cc: Paul Brook , qemu-devel@nongnu.org X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Hi, > I find stashing error messages for later printing rather awkward. Do > you provide space for one fixed-sized message? Or arbitrary length? > Arbitrary number of messages? Once you start to malloc(), you get to > worry about free()... Meh. Arbitrary length. See attached patch. > I'd rather use the global or thread-local state to hold the sink for the > messages, then send the messages there as we make them. No memory > management worries. i.e. like config_error() in net.c? What I don't like there is that the error handling policy (monitor -> continue, otherwise exit) is in the error printing function. IMHO it is the job of the caller to decide how to handle an error (exit, ignore, pass up, whatever). cheers, Gerd diff --git a/Makefile b/Makefile index 5279504..e1e85f4 100644 --- a/Makefile +++ b/Makefile @@ -87,7 +87,7 @@ obj-y += sd.o ssi-sd.o obj-y += bt.o bt-host.o bt-vhci.o bt-l2cap.o bt-sdp.o bt-hci.o bt-hid.o usb-bt.o obj-y += bt-hci-csr.o obj-y += buffered_file.o migration.o migration-tcp.o net.o qemu-sockets.o -obj-y += qemu-char.o aio.o net-checksum.o savevm.o +obj-y += qemu-char.o qemu-log.o aio.o net-checksum.o savevm.o obj-y += msmouse.o ps2.o obj-y += qdev.o qdev-properties.o ssi.o diff --git a/hw/qdev.c b/hw/qdev.c index 1b7d963..53c0117 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -29,6 +29,7 @@ #include "qdev.h" #include "sysemu.h" #include "monitor.h" +#include "qemu-log.h" /* This is a nasty hack to allow passing a NULL bus to qdev_create. */ static BusState *main_system_bus; @@ -92,7 +93,8 @@ DeviceState *qdev_create(BusState *bus, const char *name) info = qdev_find_info(bus->info, name); if (!info) { - hw_error("Unknown device '%s' for bus '%s'\n", name, bus->info->name); + error_append("Unknown device '%s' for bus '%s'\n", name, bus->info->name); + return NULL; } dev = qemu_mallocz(info->size); @@ -138,8 +140,8 @@ static int set_property(const char *name, const char *value, void *opaque) return 0; if (-1 == qdev_prop_parse(dev, name, value)) { - fprintf(stderr, "can't set property \"%s\" to \"%s\" for \"%s\"\n", - name, value, dev->info->name); + error_append("can't set property \"%s\" to \"%s\" for \"%s\"\n", + name, value, dev->info->name); return -1; } return 0; @@ -154,14 +156,14 @@ DeviceState *qdev_device_add(QemuOpts *opts) driver = qemu_opt_get(opts, "driver"); if (!driver) { - fprintf(stderr, "-device: no driver specified\n"); + error_append("-device: no driver specified\n"); return NULL; } if (strcmp(driver, "?") == 0) { char msg[256]; for (info = device_info_list; info != NULL; info = info->next) { qdev_print_devinfo(info, msg, sizeof(msg)); - fprintf(stderr, "%s\n", msg); + error_append("%s\n", msg); } return NULL; } @@ -169,13 +171,13 @@ DeviceState *qdev_device_add(QemuOpts *opts) /* find driver */ info = qdev_find_info(NULL, driver); if (!info) { - fprintf(stderr, "Device \"%s\" not found. Try -device '?' for a list.\n", - driver); + error_append("Device \"%s\" not found. Try -device '?' for a list.\n", + driver); return NULL; } if (info->no_user) { - fprintf(stderr, "device \"%s\" can't be added via command line\n", - info->name); + error_append("device \"%s\" can't be added via command line\n", + info->name); return NULL; } @@ -191,6 +193,9 @@ DeviceState *qdev_device_add(QemuOpts *opts) /* create device, set properties */ qdev = qdev_create(bus, driver); + if (qdev == NULL) { + return NULL; + } id = qemu_opts_id(opts); if (id) { qdev->id = id; diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index 71c3868..f33c97a 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -20,6 +20,7 @@ #include "sysemu.h" #include "msix.h" #include "net.h" +#include "qemu-log.h" /* from Linux's linux/virtio_pci.h */ @@ -434,7 +435,7 @@ static int virtio_blk_init_pci(PCIDevice *pci_dev) proxy->class_code = PCI_CLASS_STORAGE_SCSI; if (!proxy->dinfo) { - fprintf(stderr, "drive property not set\n"); + error_append("virtio-blk-pci: drive property not set\n"); return -1; } vdev = virtio_blk_init(&pci_dev->qdev, proxy->dinfo); diff --git a/qemu-log.c b/qemu-log.c new file mode 100644 index 0000000..0a5d81f --- /dev/null +++ b/qemu-log.c @@ -0,0 +1,60 @@ +#include "qemu-common.h" +#include "monitor.h" +#include "qemu-log.h" + +/* message buffer implementation */ + +struct MsgBuf { + unsigned int bufsize; + unsigned int msgsize; + char *message; +}; + +void msg_append(MsgBuf *buf, const char *fmt, ...) +{ + va_list args; + size_t len; + +again: + va_start(args, fmt); + len = vsnprintf(buf->message + buf->msgsize, + buf->bufsize - buf->msgsize, fmt, args); + va_end(args); + if (len > buf->bufsize - buf->msgsize) { + buf->bufsize = buf->msgsize + len + 1; + buf->message = qemu_realloc(buf->message, buf->bufsize); + goto again; + } + buf->msgsize += len; +} + +void msg_clear(MsgBuf *buf) +{ + buf->msgsize = 0; +} + +void msg_free(MsgBuf *buf) +{ + free(buf->message); + buf->message = NULL; + buf->msgsize = 0; + buf->bufsize = 0; +} + +void msg_print_file(MsgBuf *buf, FILE *fp, int clear) +{ + fprintf(fp, "%.*s", buf->msgsize, buf->message); + if (clear) + msg_clear(buf); +} + +void msg_print_mon(MsgBuf *buf, Monitor *mon, int clear) +{ + monitor_printf(mon, "%.*s", buf->msgsize, buf->message); + if (clear) + msg_clear(buf); +} + +/* use message buffer to store error messages */ + +struct MsgBuf qemu_error_message; diff --git a/qemu-log.h b/qemu-log.h index fccfb11..0dea895 100644 --- a/qemu-log.h +++ b/qemu-log.h @@ -51,9 +51,9 @@ extern int loglevel; /* Special cases: */ /* cpu_dump_state() logging functions: */ -#define log_cpu_state(env, f) cpu_dump_state((env), logfile, fprintf, (f)); -#define log_cpu_state_mask(b, env, f) do { \ - if (loglevel & (b)) log_cpu_state((env), (f)); \ +#define log_cpu_state(_env, f) cpu_dump_state((_env), logfile, fprintf, (f)); +#define log_cpu_state_mask(b, _env, f) do { \ + if (loglevel & (b)) log_cpu_state((_env), (f)); \ } while (0) /* disas() and target_disas() to logfile: */ @@ -90,4 +90,24 @@ extern int loglevel; } while (0) +/* message buffer implementation */ +struct Monitor; +typedef struct MsgBuf MsgBuf; + +void msg_append(MsgBuf *buf, const char *fmt, ...) + __attribute__ ((format(printf, 2, 3))); +void msg_clear(MsgBuf *buf); +void msg_free(MsgBuf *buf); +void msg_print_file(MsgBuf *buf, FILE *fp, int clear); +void msg_print_mon(MsgBuf *buf, struct Monitor *mon, int clear); + +/* use message buffer to store error messages */ +extern struct MsgBuf qemu_error_message; +#define error_append(...) \ + msg_append(&qemu_error_message, ## __VA_ARGS__) +#define error_print_stderr() \ + msg_print_file(&qemu_error_message, stderr, 1) +#define error_print_monitor(mon) \ + msg_print_mon(&qemu_error_message, mon, 1) + #endif diff --git a/vl.c b/vl.c index 8b2b289..b88afbe 100644 --- a/vl.c +++ b/vl.c @@ -5936,8 +5936,10 @@ int main(int argc, char **argv, char **envp) } /* init generic devices */ - if (qemu_opts_foreach(&qemu_device_opts, device_init_func, NULL, 1) != 0) + if (qemu_opts_foreach(&qemu_device_opts, device_init_func, NULL, 1) != 0) { + error_print_stderr(); exit(1); + } if (!display_state) dumb_display_init();