Re: [Qemu-devel] [RFC PATCH v4 00/30] ACPI memory hotplug
在 2013-01-09三的 01:08 +0100,Andreas Färber写道: > Am 18.12.2012 13:41, schrieb Vasilis Liaskovitis: > > Because dimm layout needs to be configured on machine-boot, all dimm devices > > need to be specified on startup command line (either with populated=on or > > with > > populated=off). The dimm information is stored in dimm configuration > > structures. > > > > After machine startup, dimms are hot-added or removed with normal device_add > > and device_del operations e.g.: > > Hot-add syntax: "device_add dimm,id=mydimm0,bus=membus.0" > > Hot-remove syntax: "device_del dimm,id=mydimm0" > > This sounds contradictory: Either all devices need to be specified on > the command line, or they can be hot-added via monitor. > > Assuming a fixed layout at startup, I wonder if there is another clever > way to model this... For CPU hotplug Anthony had suggested to have a > fixed set of link properties that get set to a CPU socket as > needed. Might a similar strategy work for memory, i.e. a > startup-configured amount of links on /machine/dimm[n] that point > to a QOM DIMM object or NULL if unpopulated? Hot(un)plug would then > simply work via QMP qom-set command. (CC'ing some people) Sorry, what's link<>, did it adopted by cpu-QOM? can you give some hints? Thanks!
Re: [Qemu-devel] [RFC PATCH v4 07/30] Add SIZE type to qdev properties
在 2012-12-18二的 13:41 +0100,Vasilis Liaskovitis写道: > This patch adds a 'SIZE' type property to qdev. > > It will make dimm description more convenient by allowing sizes to be > specified > with K,M,G,T prefixes instead of number of bytes e.g.: > -device dimm,id=mem0,size=2G,bus=membus.0 > > Credits go to Ian Molton for original patch. See: > http://patchwork.ozlabs.org/patch/38835/ > > Signed-off-by: Vasilis Liaskovitis > --- > hw/qdev-properties.c | 60 > ++ > hw/qdev-properties.h |3 ++ > qemu-option.c|2 +- > qemu-option.h|2 + > 4 files changed, 66 insertions(+), 1 deletions(-) > > diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c > index 81d901c..a77f760 100644 > --- a/hw/qdev-properties.c > +++ b/hw/qdev-properties.c > @@ -1279,3 +1279,63 @@ void qemu_add_globals(void) > { > qemu_opts_foreach(qemu_find_opts("global"), qdev_add_one_global, NULL, > 0); > } > + > +/* --- 64bit unsigned int 'size' type --- */ > + > +static void get_size(Object *obj, Visitor *v, void *opaque, > + const char *name, Error **errp) > +{ > +DeviceState *dev = DEVICE(obj); > +Property *prop = opaque; > +uint64_t *ptr = qdev_get_prop_ptr(dev, prop); > + > +visit_type_size(v, ptr, name, errp); > +} > + > +static void set_size(Object *obj, Visitor *v, void *opaque, > + const char *name, Error **errp) > +{ > +DeviceState *dev = DEVICE(obj); > +Property *prop = opaque; > +uint64_t *ptr = qdev_get_prop_ptr(dev, prop); > + > +if (dev->state != DEV_STATE_CREATED) { > +error_set(errp, QERR_PERMISSION_DENIED); > +return; > +} > + > +visit_type_size(v, ptr, name, errp); > +} > + > +static int parse_size(DeviceState *dev, Property *prop, const char *str) > +{ > +uint64_t *ptr = qdev_get_prop_ptr(dev, prop); > +Error *errp = NULL; > + > +if (str != NULL) { > +parse_option_size(prop->name, str, ptr, &errp); > +} > +assert_no_error(errp); > +return 0; > +} > + > +static int print_size(DeviceState *dev, Property *prop, char *dest, size_t > len) > +{ > +uint64_t *ptr = qdev_get_prop_ptr(dev, prop); > +char suffixes[] = {'T', 'G', 'M', 'K', 'B'}; > +int i = 0; > +uint64_t div; > + > +for (div = (long int)1 << 40; !(*ptr / div) ; div >>= 10) { > +i++; > +} > +return snprintf(dest, len, "%0.03f%c", (double)*ptr/div, suffixes[i]); ^^ ^^^ > +} > + IMHO, you may need (double)(*ptr/div), for type cast is right associated. > +PropertyInfo qdev_prop_size = { > +.name = "size", > +.parse = parse_size, > +.print = print_size, > +.get = get_size, > +.set = set_size, > +}; > diff --git a/hw/qdev-properties.h b/hw/qdev-properties.h > index 5b046ab..0182bef 100644 > --- a/hw/qdev-properties.h > +++ b/hw/qdev-properties.h > @@ -14,6 +14,7 @@ extern PropertyInfo qdev_prop_uint64; > extern PropertyInfo qdev_prop_hex8; > extern PropertyInfo qdev_prop_hex32; > extern PropertyInfo qdev_prop_hex64; > +extern PropertyInfo qdev_prop_size; > extern PropertyInfo qdev_prop_string; > extern PropertyInfo qdev_prop_chr; > extern PropertyInfo qdev_prop_ptr; > @@ -67,6 +68,8 @@ extern PropertyInfo qdev_prop_pci_host_devaddr; > DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex32, uint32_t) > #define DEFINE_PROP_HEX64(_n, _s, _f, _d) \ > DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex64, uint64_t) > +#define DEFINE_PROP_SIZE(_n, _s, _f, _d) \ > +DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_size, uint64_t) > #define DEFINE_PROP_PCI_DEVFN(_n, _s, _f, _d) \ > DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_pci_devfn, int32_t) > > diff --git a/qemu-option.c b/qemu-option.c > index 27891e7..38e0a11 100644 > --- a/qemu-option.c > +++ b/qemu-option.c > @@ -203,7 +203,7 @@ static void parse_option_number(const char *name, const > char *value, > } > } > > -static void parse_option_size(const char *name, const char *value, > +void parse_option_size(const char *name, const char *value, >uint64_t *ret, Error **errp) > { > char *postfix; > diff --git a/qemu-option.h b/qemu-option.h > index ca72986..b8ee5b3 100644 > --- a/qemu-option.h > +++ b/qemu-option.h > @@ -152,5 +152,7 @@ typedef int (*qemu_opts_loopfunc)(QemuOpts *opts, void > *opaque); > int qemu_opts_print(QemuOpts *opts, void *dummy); > int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void > *opaque, >int abort_on_failure); > +void parse_option_size(const char *name, const char *value, > + uint64_t *ret, Error **errp); > > #endif
Re: [Qemu-devel] [RESEND PATCH v2] pciinit: Enable default VGA device
Hi, > Turns out it's this: > > commit 76e58028d28e78431f9de3cee0b3c88d807fa39d > Author: Kevin O'Connor > Date: Wed Mar 6 21:50:09 2013 -0500 > > acpi: Eliminate BDAT parameter passing to DSDT code. > > The "BDAT" construct is the only ACPI mechanism that relies on SeaBIOS > reserved memory. Replace it with the SSDT based template system. > > Signed-off-by: Kevin O'Connor That is most likely dsdt + bios.bin not being in sync. Simply using 'qemu -bios /path/to/seabios/out/bios.bin' doesn't fly due to qemu using a non-matching dsdt then. With qemu/master you can just use 'qemu -L /path/to/seabios/out' instead and qemu will pick up both bios.bin and dsdt from the fresh seabios build directory then (and anything else it doesn't find there from the default locations). HTH, Gerd
[Qemu-devel] [PATCH 3/3] virtio-ccw, s390-virtio: Use generic virtio-blk macro.
Now that virtio-ccw and s390-virtio define all common properties for virtio-blk, we can switch to using the generic DEFINE_VIRTIO_BLK_PROPERTIES macro. CC: Alexander Graf Signed-off-by: Cornelia Huck --- hw/s390x/s390-virtio-bus.c |8 +--- hw/s390x/virtio-ccw.c |8 +--- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c index 5d55a9d..c5d5456 100644 --- a/hw/s390x/s390-virtio-bus.c +++ b/hw/s390x/s390-virtio-bus.c @@ -435,13 +435,7 @@ static const TypeInfo s390_virtio_net = { }; static Property s390_virtio_blk_properties[] = { -DEFINE_BLOCK_PROPERTIES(VirtIOBlkS390, blk.conf), -DEFINE_BLOCK_CHS_PROPERTIES(VirtIOBlkS390, blk.conf), -DEFINE_PROP_STRING("serial", VirtIOBlkS390, blk.serial), -#ifdef __linux__ -DEFINE_PROP_BIT("scsi", VirtIOBlkS390, blk.scsi, 0, true), -#endif -DEFINE_PROP_BIT("config-wce", VirtIOBlkS390, blk.config_wce, 0, true), +DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkS390, blk), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index afaf348..4c44b7e 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -756,14 +756,8 @@ static const TypeInfo virtio_ccw_net = { static Property virtio_ccw_blk_properties[] = { DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id), -DEFINE_BLOCK_PROPERTIES(VirtIOBlkCcw, blk.conf), -DEFINE_BLOCK_CHS_PROPERTIES(VirtIOBlkCcw, blk.conf), -DEFINE_PROP_STRING("serial", VirtIOBlkCcw, blk.serial), -#ifdef __linux__ -DEFINE_PROP_BIT("scsi", VirtIOBlkCcw, blk.scsi, 0, true), -#endif -DEFINE_PROP_BIT("config-wce", VirtIOBlkCcw, blk.config_wce, 0, true), DEFINE_VIRTIO_BLK_FEATURES(VirtioCcwDevice, host_features[0]), +DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkCcw, blk), DEFINE_PROP_END_OF_LIST(), }; -- 1.7.9.5
[Qemu-devel] [PATCH 1/3] virtio-ccw: Add missing blk chs properties.
Signed-off-by: Cornelia Huck --- hw/s390x/virtio-ccw.c |1 + 1 file changed, 1 insertion(+) diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 9688835..fc95116 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -757,6 +757,7 @@ static const TypeInfo virtio_ccw_net = { static Property virtio_ccw_blk_properties[] = { DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id), DEFINE_BLOCK_PROPERTIES(VirtIOBlkCcw, blk.conf), +DEFINE_BLOCK_CHS_PROPERTIES(VirtIOBlkCcw, blk.conf), DEFINE_PROP_STRING("serial", VirtIOBlkCcw, blk.serial), #ifdef __linux__ DEFINE_PROP_BIT("scsi", VirtIOBlkCcw, blk.scsi, 0, true), -- 1.7.9.5
[Qemu-devel] [PATCH 0/3] virtio-ccw queue as of 2013/03/20
Hi, here are again my patches adding missing virtio-blk properties for the s390 transports, this time rebased on top of the virtio-blk refactoring. The third patch is new, bringing s390-virtio and virtio-ccw in line with virtio-pci. Alex, again I'd like an ack from you (for the new patch) so I can take this through my tree. Cornelia Huck (3): virtio-ccw: Add missing blk chs properties. s390-virtio, virtio-ccw: Add config_wce for virtio-blk. virtio-ccw, s390-virtio: Use generic virtio-blk macro. hw/s390x/s390-virtio-bus.c |7 +-- hw/s390x/virtio-ccw.c |6 +- 2 files changed, 2 insertions(+), 11 deletions(-) -- 1.7.9.5
[Qemu-devel] [PATCH 2/3] s390-virtio, virtio-ccw: Add config_wce for virtio-blk.
There's no reason why we wouldn't want to make the cache mode configurable. Acked-by: Alexander Graf Signed-off-by: Cornelia Huck --- hw/s390x/s390-virtio-bus.c |1 + hw/s390x/virtio-ccw.c |1 + 2 files changed, 2 insertions(+) diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c index 76bc99a..5d55a9d 100644 --- a/hw/s390x/s390-virtio-bus.c +++ b/hw/s390x/s390-virtio-bus.c @@ -441,6 +441,7 @@ static Property s390_virtio_blk_properties[] = { #ifdef __linux__ DEFINE_PROP_BIT("scsi", VirtIOBlkS390, blk.scsi, 0, true), #endif +DEFINE_PROP_BIT("config-wce", VirtIOBlkS390, blk.config_wce, 0, true), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index fc95116..afaf348 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -762,6 +762,7 @@ static Property virtio_ccw_blk_properties[] = { #ifdef __linux__ DEFINE_PROP_BIT("scsi", VirtIOBlkCcw, blk.scsi, 0, true), #endif +DEFINE_PROP_BIT("config-wce", VirtIOBlkCcw, blk.config_wce, 0, true), DEFINE_VIRTIO_BLK_FEATURES(VirtioCcwDevice, host_features[0]), DEFINE_PROP_END_OF_LIST(), }; -- 1.7.9.5
Re: [Qemu-devel] [PATCH] virtio-blk: Set default serial id
On Wed, Mar 20, 2013 at 01:56:08PM +0800, Asias He wrote: > If user does not specify a serial id, e.g. > >-device virtio-blk-pci,serial="serial_id" > or >-drive serial="serial_id" > > no serial id will be assigned. > > Add a default serial id in this case to help identifying > the disk in guest. > > Signed-off-by: Asias He > --- > hw/virtio-blk.c | 7 +++ > 1 file changed, 7 insertions(+) Autogenerated IDs have been proposed (for other devices?) before and I think we should avoid them. The serial in this patch depends on the internal counter we use for savevm. It is not a well-defined value that guests can depend on remaining the same. It can change between QEMU invocations - due to internal changes in QEMU or because the management tool reordered -device options. Users will be confused and their guests may stop working if they depend on an ID like this. The solution is to do persistent naming either by really passing -device virtio-blk-pci,serial= or with udev inside the guest using the bus address (PCI devfn) like the new persistent network interface naming for Linux. Stefan
Re: [Qemu-devel] [PATCH 0/3] virtio-ccw queue as of 2013/03/20
On 20/03/2013 08:43, Cornelia Huck wrote: Hi, here are again my patches adding missing virtio-blk properties for the s390 transports, this time rebased on top of the virtio-blk refactoring. The third patch is new, bringing s390-virtio and virtio-ccw in line with virtio-pci. Alex, again I'd like an ack from you (for the new patch) so I can take this through my tree. Cornelia Huck (3): virtio-ccw: Add missing blk chs properties. s390-virtio, virtio-ccw: Add config_wce for virtio-blk. virtio-ccw, s390-virtio: Use generic virtio-blk macro. hw/s390x/s390-virtio-bus.c |7 +-- hw/s390x/virtio-ccw.c |6 +- 2 files changed, 2 insertions(+), 11 deletions(-) Can I be CC'ed when this goes in? Thanks, Fred
Re: [Qemu-devel] [PATCH 0/3] virtio-ccw queue as of 2013/03/20
On Wed, 20 Mar 2013 09:03:13 +0100 KONRAD Frédéric wrote: > On 20/03/2013 08:43, Cornelia Huck wrote: > > Hi, > > > > here are again my patches adding missing virtio-blk properties > > for the s390 transports, this time rebased on top of the virtio-blk > > refactoring. > > > > The third patch is new, bringing s390-virtio and virtio-ccw in line > > with virtio-pci. > > > > Alex, again I'd like an ack from you (for the new patch) so I can > > take this through my tree. > > > > Cornelia Huck (3): > >virtio-ccw: Add missing blk chs properties. > >s390-virtio, virtio-ccw: Add config_wce for virtio-blk. > >virtio-ccw, s390-virtio: Use generic virtio-blk macro. > > > > hw/s390x/s390-virtio-bus.c |7 +-- > > hw/s390x/virtio-ccw.c |6 +- > > 2 files changed, 2 insertions(+), 11 deletions(-) > > > Can I be CC'ed when this goes in? Sure. > > Thanks, > Fred >
Re: [Qemu-devel] kvm suspend performance
On Tue, Mar 19, 2013 at 05:24:59PM +0100, Thomas Knauth wrote: > lately I've been playing around with qemu's/kvm's suspend (to disk) and > resume. My initial expectation was that both operations are I/O bound. So > it surprised me to see that suspend to disk seems to be CPU-bound. > Suspending a VM with 1.5 GB memory takes 55 seconds. This works out to less > than 30 MB/s. Again, I was expecting to be I/O bound and reach 100 MB/s > (and more). I am talking to qemu/kvm via libvirt. Not sure if this matters. > > I am looking for a hint what the issue could be here. Hopefully with > pointers to the (offending) code. Hi Thomas, Please provide more details about the issue: Which QEMU or libvirt command are you using to suspend the guest to disk? Why do you say it is CPU-bound? Did you use a tool like vmstat or simply because it does 30 MB/s instead of the expected 100 MB/s? Which versions of libvirt and QEMU are you using? Stefan
Re: [Qemu-devel] OOM don't like qemu-kvm
On Wed, Mar 20, 2013 at 01:16:54AM +0800, Peter Cheung wrote: > Hi AllMy compuer is i7 with 32 GB ram without swap, i try to launch 28 vm > with 1GB ram each, it runs smoothly.If I launch 64 VMs, after the VMs eat up > all the memory, the linux hang. If the linux is lack of memory, the OOM will > try to kill process, right? And the OOM should pick those qemu-kvm processes > to kill, but seems the OOM don't pick qemu-kvm or the OOM failed to kill > qemu-kvm processes. > If I turn on swap, although 64 VMs with 1GB ram each will make my > computer very slow, but at least it won't hang. > Can anybody explain this? Did you check dmesg for error messages from the kernel? If the system is locked up you can use the SysRq key combinations to print out the list of tasks, list of currently held locks, force the OOM killer to run, and print backtraces for all CPUs. http://en.wikipedia.org/wiki/Magic_SysRq_key This should allow you to debug the issue further. If you decide it's a kernel issue, then k...@vger.kernel.org or the Linux kernel mailing list would be more appropriate than qemu-devel. Stefan
Re: [Qemu-devel] Requesting a wiki account.
On Tue, Mar 19, 2013 at 07:03:24PM -0400, spectre wrote: > I've been doing background reading on the wiki and have found multiple dead > links / errors I'd like to fix. Could an admin email me off list regarding > a wiki account under a different email address / name ? Hi, I will create an account for you. Contacting you off-list. Stefan
Re: [Qemu-devel] [PATCH v3] NVMe: Initial commit
On Tue, Mar 19, 2013 at 01:41:53PM -0600, Keith Busch wrote: > +static int nvme_init(PCIDevice *pci_dev) > +{ > +NvmeCtrl *n = NVME(pci_dev); > +NvmeIdCtrl *id = &n->id_ctrl; > + > +int i; > +int64_t bs_size; > +char serial[sizeof(id->sn)]; > +uint8_t *pci_conf; > + > +if (!(n->conf.bs)) { > +return -1; > +} > + > +bs_size = bdrv_getlength(n->conf.bs); > +if (bs_size <= 0) { > +return -1; > +} > + > +n->instance = instance++; > +blkconf_serial(&n->conf, &n->serial); > +if (!n->serial) { > +snprintf((char *)serial, sizeof(serial), "NVMeQx10%02x", > n->instance); > +} A patch that adds serial autogeneration to virtio-blk was recently posted. Here were my concerns: Autogenerated IDs have been proposed (for other devices?) before and I think we should avoid them. The autogenerated serial is not persistent. It can change between QEMU invocations - due to internal changes in QEMU or because the management tool reordered -device options. Users will be confused and their guests may stop working if they depend on an ID like this. The solution is to do persistent naming either by really passing -drive serial= or with udev inside the guest using the bus address (PCI devfn) like the new persistent network interface naming for Linux. Stefan
Re: [Qemu-devel] [PATCH] virtio-blk: Set default serial id
On Wed, Mar 20, 2013 at 08:52:57AM +0100, Stefan Hajnoczi wrote: > On Wed, Mar 20, 2013 at 01:56:08PM +0800, Asias He wrote: > > If user does not specify a serial id, e.g. > > > >-device virtio-blk-pci,serial="serial_id" > > or > >-drive serial="serial_id" > > > > no serial id will be assigned. > > > > Add a default serial id in this case to help identifying > > the disk in guest. > > > > Signed-off-by: Asias He > > --- > > hw/virtio-blk.c | 7 +++ > > 1 file changed, 7 insertions(+) > > Autogenerated IDs have been proposed (for other devices?) before and I > think we should avoid them. > > The serial in this patch depends on the internal counter we use for > savevm. It is not a well-defined value that guests can depend on > remaining the same. > > It can change between QEMU invocations - due to internal changes in QEMU > or because the management tool reordered -device options. > > Users will be confused and their guests may stop working if they depend > on an ID like this. So, no serial id forces user not to depend on the serial id. This is how it works now. Okay, let's leave it as it is, persistent id specified by user or no id at all. This is not perfect but at least it does not break any thing. > The solution is to do persistent naming either by really passing -device > virtio-blk-pci,serial= or with udev inside the guest using the bus > address (PCI devfn) like the new persistent network interface naming for > Linux. '-virtio-blk-pci,serial=' specified by user would be persistent, but pci id might be changed as well. > Stefan -- Asias
Re: [Qemu-devel] [PATCH] qemu-sockets: Fix assertion failure
Am 19.03.2013 um 21:34 hat Luiz Capitulino geschrieben: > On Wed, 06 Mar 2013 15:46:45 +0100 > Laszlo Ersek wrote: > > > On 03/06/13 12:11, Kevin Wolf wrote: > > > Am 06.03.2013 um 12:04 hat Paolo Bonzini geschrieben: > > >> Il 06/03/2013 11:48, Kevin Wolf ha scritto: > > >>> inet_connect_opts() tries all possible addrinfos returned by > > >>> getaddrinfo(). If one fails with an error, the next one is tried. In > > >>> this case, the Error should be discarded because the whole operation is > > >>> successful if another addrinfo from the list succeeds; and if it > > >>> doesn't, setting an already set Error will trigger an assertion failure. > > >>> > > >>> Signed-off-by: Kevin Wolf > > >>> --- > > >>> util/qemu-sockets.c | 8 > > >>> 1 file changed, 8 insertions(+) > > >>> > > >>> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c > > >>> index 1350ccc..32e609a 100644 > > >>> --- a/util/qemu-sockets.c > > >>> +++ b/util/qemu-sockets.c > > >>> @@ -373,6 +373,14 @@ int inet_connect_opts(QemuOpts *opts, Error **errp, > > >>> } > > >>> > > >>> for (e = res; e != NULL; e = e->ai_next) { > > >>> + > > >>> +/* Overwriting errors isn't allowed, so clear any error that > > >>> may have > > >>> + * occured in the previous iteration */ > > >>> +if (error_is_set(errp)) { > > >>> +error_free(*errp); > > >>> +*errp = NULL; > > >>> +} > > >>> + > > >>> if (connect_state != NULL) { > > >>> connect_state->current_addr = e; > > >>> } > > >>> > > >> > > >> Should we also do nothing if errp is not NULL on entry? > > > > > > We could assert(!error_is_set(errp)) if we wanted. As soon as you've got > > > an Error, you must return instead of calling more functions with the > > > same error pointer. > > > > I think Luiz would suggest (*) to receive any error into a > > NULL-initialized local_err pointer; do the logic above on local_err, and > > just before returning, error_propagate() it to errp. > > Yes, I'd suggest that but it turns out that inet_connect_addr() error > reporting was and still is confusing, which causes callers to use it > incorrectly. > > This patch (which has been applied by Anthony) No, Anthony applied a different, but similar patch of his own. This is why I don't feel particularly responsible for the specific problem any more. How to do error handling with Error right is the only reason for me to continue the discussion. > solves the problem at > hand but it also introduces a new issue: errors from inet_connect_addr() > are only reported if they happen in the last loop interaction. Note that > a few other errors other than 'couldn't connect' can happen. > Laszlo's comment seemed to have triggered a discussion around Error **, > but this really has very little to do with it: the real problem is that > inet_connect_addr() is too confusing. Maybe we need to discuss first what the intended behaviour even is. My interpretation was this: We may have several addresses to try. If one of them works, the function as a whole has succeeded and must not return an error, neither in errp nor as -errno. If none of them succeeds, the function has to return an error, and returning the error of the last attempt is as good as the error of any other attempt. > inet_connect_addr() has two users: inet_connect_opts() and wait_for_connect(), > with this patch both of them are now ignoring errors from inet_connect_addr(). > > Suggested solution: refactor inet_connect_addr() to return an errno value. > Callers use error_set() when they want to report an error upward. Doesn't change the problem that you need to know when to set a return value != 0. So it doesn't help, but you'd lose some error information. Kevin
Re: [Qemu-devel] [PATCH] HLFS driver for QEMU
On Mon, Mar 18, 2013 at 11:16 PM, Stefan Hajnoczi wrote: [...] > I looked at the Google Code project before, it looks like a repo that > a few people are hacking on. The site is developer-focussed and there > is no evidence of users. This is why I asked about the background of > the community. > Cloudxy is developing and we will popularize it. > There are references to "workshop3" and "linux lab" in the source > code. Xiyou and XUPT is http://www.xiyou.edu.cn/. > Some developers are from XUPT. > What I'm concerned about is that there are no users and the developers > leave when they graduate. In that case integrating the patches into > QEMU is wasted effort. > There are some developers from XUPT but not *ALL*. Be sure that our key developers will maintain this project forever like Kang Hua , Zhang Dianbo , Wang Sen and Harry Wei . We will broadcast our project so that there will be more users. > So what *is* the background of cloudxy (HLFS)? Be sure that we have enough developers. Users will be more if Cloudxy has more influences. -- Thanks Harry Wei
Re: [Qemu-devel] [PATCH 09/11] block: Make find_image_format safe with NULL filename
Am 20.03.2013 um 03:14 hat Eric Blake geschrieben: > On 03/18/2013 11:23 AM, Kevin Wolf wrote: > > In order to achieve this, the .bdrv_probe callbacks of all drivers must > > cope with this. The DMG driver is the only one that bases its decision > > on the filename and it needs to be changed. > > > > Signed-off-by: Kevin Wolf > > --- > > block/dmg.c | 13 ++--- > > 1 file changed, 10 insertions(+), 3 deletions(-) > > Reviewed-by: Eric Blake > > > > > diff --git a/block/dmg.c b/block/dmg.c > > index c1066df..3141cb5 100644 > > --- a/block/dmg.c > > +++ b/block/dmg.c > > @@ -51,9 +51,16 @@ typedef struct BDRVDMGState { > > > > static int dmg_probe(const uint8_t *buf, int buf_size, const char > > *filename) > > { > > -int len=strlen(filename); > > -if(len>4 && !strcmp(filename+len-4,".dmg")) > > Someone really didn't like whitespace :) Glad to see it rewritten to > keep the patch checker happy. Actually I don't think it would have complained, because it would be context only. But I simply can't leave this code alone while touching the function. :-) Kevin
Re: [Qemu-devel] [Xen-devel] [PATCHv3] QEMU(upstream): Disable xen's use of O_DIRECT by default as it results in crashes.
Stefano, --On 19 March 2013 15:13:29 + Stefano Stabellini wrote: Therefore I think that the current change is not safe, but it is pretty easy to make it safe. You just need to move the call to blk_open from blk_init to blk_connect. Actually you can move most of blk_init to blk_connect, you just need to keep the 4 xenstore_write_be_int at the end of the function. The final of these 4 writes in blk_init is: xenstore_write_be_int(&blkdev->xendev, "sectors", blkdev->file_size / blkdev->file_blk); and blkdev->filesize comes from bdrv_getlength(blkdev->bs), and that requires (I think) bdrv_open to have been called. Can these 4 writes move safely to blk_connect? Or can we write the sectors count as (say) 0 here and fix it later in blk_connect? The remainder look ok. -- Alex Bligh
Re: [Qemu-devel] [PATCH 08/11] block: Rename variable to avoid shadowing
Am 20.03.2013 um 03:07 hat Eric Blake geschrieben: > On 03/18/2013 11:23 AM, Kevin Wolf wrote: > > bdrv_open() uses two different variables called options. Rename one of > > them to avoid confusion and to allow the outer one to be accessed > > everywhere. > > > > Signed-off-by: Kevin Wolf > > --- > > block.c | 16 +--- > > 1 file changed, 9 insertions(+), 7 deletions(-) > > Reviewed-by: Eric Blake > > Should configure be setting up -Wshadow to help prevent instances like this? Perhaps it should. But I tried it out and there are by far too many warnings, so that doing this patch wouldn't be a trivial task. Kevin
[Qemu-devel] [PATCH] virtio-blk-x: fix configuration synchronization.
From: KONRAD Frederic The virtio-blk-x configuration is not in sync with virtio-blk configuration. So this patch remove the virtio-blk-x configuration field, and use virtio-blk one for setting the properties. This also remove a useless configuration copy in virtio_blk_device_init. Note that this is on top of "virtio-ccw queue as of 2013/03/20". Signed-off-by: KONRAD Frederic --- hw/s390x/s390-virtio-bus.c | 4 ++-- hw/s390x/s390-virtio-bus.h | 1 - hw/s390x/virtio-ccw.c | 4 ++-- hw/s390x/virtio-ccw.h | 1 - hw/virtio-blk.c| 7 --- hw/virtio-blk.h| 2 -- hw/virtio-pci.c| 7 --- hw/virtio-pci.h| 1 - 8 files changed, 8 insertions(+), 19 deletions(-) diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c index c5d5456..b6f9682 100644 --- a/hw/s390x/s390-virtio-bus.c +++ b/hw/s390x/s390-virtio-bus.c @@ -166,7 +166,7 @@ static int s390_virtio_blk_init(VirtIOS390Device *s390_dev) { VirtIOBlkS390 *dev = VIRTIO_BLK_S390(s390_dev); DeviceState *vdev = DEVICE(&dev->vdev); -virtio_blk_set_conf(vdev, &(dev->blk)); + qdev_set_parent_bus(vdev, BUS(&s390_dev->bus)); if (qdev_init(vdev) < 0) { return -1; @@ -435,7 +435,7 @@ static const TypeInfo s390_virtio_net = { }; static Property s390_virtio_blk_properties[] = { -DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkS390, blk), +DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkS390, vdev.blk), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/s390x/s390-virtio-bus.h b/hw/s390x/s390-virtio-bus.h index 1a63411..05347bc 100644 --- a/hw/s390x/s390-virtio-bus.h +++ b/hw/s390x/s390-virtio-bus.h @@ -128,7 +128,6 @@ void s390_virtio_reset_idx(VirtIOS390Device *dev); typedef struct VirtIOBlkS390 { VirtIOS390Device parent_obj; VirtIOBlock vdev; -VirtIOBlkConf blk; } VirtIOBlkS390; diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 4c44b7e..4a10bd6 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -574,7 +574,7 @@ static int virtio_ccw_blk_init(VirtioCcwDevice *ccw_dev) { VirtIOBlkCcw *dev = VIRTIO_BLK_CCW(ccw_dev); DeviceState *vdev = DEVICE(&dev->vdev); -virtio_blk_set_conf(vdev, &(dev->blk)); + qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus)); if (qdev_init(vdev) < 0) { return -1; @@ -757,7 +757,7 @@ static const TypeInfo virtio_ccw_net = { static Property virtio_ccw_blk_properties[] = { DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id), DEFINE_VIRTIO_BLK_FEATURES(VirtioCcwDevice, host_features[0]), -DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkCcw, blk), +DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkCcw, vdev.blk), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h index 3993bc5..8f7c3a5 100644 --- a/hw/s390x/virtio-ccw.h +++ b/hw/s390x/virtio-ccw.h @@ -102,7 +102,6 @@ typedef struct VirtualCssBus { typedef struct VirtIOBlkCcw { VirtioCcwDevice parent_obj; VirtIOBlock vdev; -VirtIOBlkConf blk; } VirtIOBlkCcw; diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index f2143fd..0414a8a 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -619,12 +619,6 @@ static const BlockDevOps virtio_block_ops = { .resize_cb = virtio_blk_resize, }; -void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk) -{ -VirtIOBlock *s = VIRTIO_BLK(dev); -memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf)); -} - static int virtio_blk_device_init(VirtIODevice *vdev) { DeviceState *qdev = DEVICE(vdev); @@ -656,7 +650,6 @@ static int virtio_blk_device_init(VirtIODevice *vdev) vdev->reset = virtio_blk_reset; s->bs = blk->conf.bs; s->conf = &blk->conf; -memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf)); s->rq = NULL; s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1; diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h index 8c6c78b..e228057 100644 --- a/hw/virtio-blk.h +++ b/hw/virtio-blk.h @@ -148,6 +148,4 @@ typedef struct VirtIOBlock { DEFINE_PROP_BIT("config-wce", _state, _field.config_wce, 0, true) #endif /* __linux__ */ -void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk); - #endif diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index f3ece78..31a78de 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -1405,10 +1405,11 @@ static Property virtio_blk_pci_properties[] = { VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2), #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE -DEFINE_PROP_BIT("x-data-plane", VirtIOBlkPCI, blk.data_plane, 0, false), +DEFINE_PROP_BIT("x-data-plane", VirtIOBlkPCI, vdev.blk.data_plane, 0, + false), #endif DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features), -DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkPCI, blk), +DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkPCI, vdev.blk
Re: [Qemu-devel] [PATCH v14 2/4] add a new qevent: QEVENT_GUEST_PANICKED
Hu Tao writes: > This event will be emited when the guest is panicked. > > Signed-off-by: Wen Congyang > Signed-off-by: Hu Tao > --- > QMP/qmp-events.txt| 14 ++ > include/monitor/monitor.h | 1 + > monitor.c | 1 + > 3 files changed, 16 insertions(+) > > diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt > index b2698e4..62fffad 100644 > --- a/QMP/qmp-events.txt > +++ b/QMP/qmp-events.txt > @@ -428,3 +428,17 @@ Example: > > Note: If action is "reset", "shutdown", or "pause" the WATCHDOG event is > followed respectively by the RESET, SHUTDOWN, or STOP events. > + > +GUEST_PANICKED > +-- > + > +Emitted when the guest OS is panicked. This suggests we reliably emit the event on guest panic. That's not accurate; we emit it only when the guest manages to tell us. Better clarify. Perhaps "Emitted when we detect a guest OS panic" suffices. > + > +Data: > + > +- "action": Action that has been taken. Currently it's always "pause". Please make that - "action": Action that has been taken (json-string, currently always "pause") > + > +Example: > + > +{ "event": "GUEST_PANICKED", > + "data": { "action": "pause" } } > diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h > index 87fb49c..4006905 100644 > --- a/include/monitor/monitor.h > +++ b/include/monitor/monitor.h > @@ -45,6 +45,7 @@ typedef enum MonitorEvent { > QEVENT_WAKEUP, > QEVENT_BALLOON_CHANGE, > QEVENT_SPICE_MIGRATE_COMPLETED, > +QEVENT_GUEST_PANICKED, > > /* Add to 'monitor_event_names' array in monitor.c when > * defining new events here */ > diff --git a/monitor.c b/monitor.c > index 32a6e74..5b7d7f9 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -463,6 +463,7 @@ static const char *monitor_event_names[] = { > [QEVENT_WAKEUP] = "WAKEUP", > [QEVENT_BALLOON_CHANGE] = "BALLOON_CHANGE", > [QEVENT_SPICE_MIGRATE_COMPLETED] = "SPICE_MIGRATE_COMPLETED", > +[QEVENT_GUEST_PANICKED] = "GUEST_PANICKED", > }; > QEMU_BUILD_BUG_ON(ARRAY_SIZE(monitor_event_names) != QEVENT_MAX)
Re: [Qemu-devel] [PATCH v14 1/4] add a new runstate: RUN_STATE_GUEST_PANICKED
Hu Tao writes: > The guest will be in this state when it is panicked. > > Signed-off-by: Wen Congyang > Signed-off-by: Hu Tao > --- > include/sysemu/sysemu.h | 1 + > qapi-schema.json| 5 - > qmp.c | 3 +-- > vl.c| 13 +++-- > 4 files changed, 17 insertions(+), 5 deletions(-) > > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > index b19ec95..0412a8a 100644 > --- a/include/sysemu/sysemu.h > +++ b/include/sysemu/sysemu.h > @@ -22,6 +22,7 @@ int qemu_uuid_parse(const char *str, uint8_t *uuid); > bool runstate_check(RunState state); > void runstate_set(RunState new_state); > int runstate_is_running(void); > +bool runstate_needs_reset(void); > typedef struct vm_change_state_entry VMChangeStateEntry; > typedef void VMChangeStateHandler(void *opaque, int running, RunState state); > > diff --git a/qapi-schema.json b/qapi-schema.json > index 28b070f..003cbf2 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -174,11 +174,14 @@ > # @suspended: guest is suspended (ACPI S3) > # > # @watchdog: the watchdog action is configured to pause and has been > triggered > +# > +# @guest-panicked: guest has been panicked as a result of guest OS panic > ## > { 'enum': 'RunState', >'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused', > 'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm', > -'running', 'save-vm', 'shutdown', 'suspended', 'watchdog' ] } > +'running', 'save-vm', 'shutdown', 'suspended', 'watchdog', > +'guest-panicked' ] } > > ## > # @SnapshotInfo RUN_STATE_GUEST_PANICKED is similar to RUN_STATE_INTERNAL_ERROR and RUN_STATE_SHUTDOWN: the only way for the guest to continue is reset. Correct? > diff --git a/qmp.c b/qmp.c > index 55b056b..a1ebb5d 100644 > --- a/qmp.c > +++ b/qmp.c > @@ -149,8 +149,7 @@ void qmp_cont(Error **errp) > { > Error *local_err = NULL; > > -if (runstate_check(RUN_STATE_INTERNAL_ERROR) || > - runstate_check(RUN_STATE_SHUTDOWN)) { > +if (runstate_needs_reset()) { > error_set(errp, QERR_RESET_REQUIRED); > return; > } else if (runstate_check(RUN_STATE_SUSPENDED)) { > diff --git a/vl.c b/vl.c > index c03edf1..926822b 100644 > --- a/vl.c > +++ b/vl.c > @@ -566,6 +566,7 @@ static const RunStateTransition > runstate_transitions_def[] = { > { RUN_STATE_RUNNING, RUN_STATE_SAVE_VM }, > { RUN_STATE_RUNNING, RUN_STATE_SHUTDOWN }, > { RUN_STATE_RUNNING, RUN_STATE_WATCHDOG }, > +{ RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED }, > > { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING }, > This permits runstate_set(RUN_STATE_GUEST_PANICKED) in RUN_STATE_RUNNING. Used in PATCH 3/4. Good. > @@ -580,6 +581,8 @@ static const RunStateTransition > runstate_transitions_def[] = { > { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING }, > { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE }, > > +{ RUN_STATE_GUEST_PANICKED, RUN_STATE_PAUSED }, > + > { RUN_STATE_MAX, RUN_STATE_MAX }, > }; This permits runstate_set(RUN_STATE_PAUSED) in RUN_STATE_GUEST_PANICKED. An example for such a transition is in the last patch hunk: main loop resetting the guest. Let's examine the other transitions to RUN_STATE_PAUSED, and whether they can now come from RUN_STATE_GUEST_PANICKED: * gdb_read_byte() No, because vm_stop() is protected by runstate_is_running() here. * gdb_chr_event() case CHR_EVENT_OPENED Can this happen in RUN_STATE_GUEST_PANICKED? Jan? What about RUN_STATE_INTERNAL_ERROR and RUN_STATE_SHUTDOWN? * gdb_sigterm_handler() No, because vm_stop() is protected by runstate_is_running() here. Aside: despite its name, this function handles SIGINT. Ugh. * process_incoming_migration_co() No, because we're in RUN_STATE_INMIGRATE here, aren't we? Juan? * qmp_stop() No, because vm_stop() calls do_vm_stop() to do the actual state transition, which protects it by runstate_is_running(). We can ignore the conditional, it merely punts the vm_stop() to the main loop. Next question: RUN_STATE_INTERNAL_ERROR and RUN_STATE_SHUTDOWN may go to RUN_STATE_FINISH_MIGRATE, but RUN_STATE_GUEST_PANICKED may not. Why? > > @@ -621,6 +624,13 @@ int runstate_is_running(void) > return runstate_check(RUN_STATE_RUNNING); > } > > +bool runstate_needs_reset(void) > +{ > +return runstate_check(RUN_STATE_INTERNAL_ERROR) || > +runstate_check(RUN_STATE_SHUTDOWN) || > +runstate_check(RUN_STATE_GUEST_PANICKED); > +} > + > StatusInfo *qmp_query_status(Error **errp) > { > StatusInfo *info = g_malloc0(sizeof(*info)); > @@ -1966,8 +1976,7 @@ static bool main_loop_should_exit(void) > cpu_synchronize_all_states(); > qemu_system_reset(VMRESET_REPORT); > resume_all_vcpus(); > -if (runstate_check(RUN_STATE_INTERNAL_ERROR) || > -runstate_check(RUN_STATE_SHUTDOWN)) { > +if (runstate_
[Qemu-devel] [PATCH] Fix I/O throttling pathologic oscillating behavior
Limiting a virtio device backed by a QED file at 150 iops with "-drive file=test.qed,if=virtio,cache=none,iops=150" and running in the guest the load.c program lead to an iops oscillating behavior which amplify itself as the time goes on. At first an extract of "iostats -x -d 2" would look like this: Device: rrqm/s wrqm/s r/s w/srkB/swkB/s avgrq-sz vdb 0,00 4,110,10 149,09 0,39 614,02 8,24 vdb 0,00 0,000,00 163,27 0,00 653,06 8,00 vdb 0,00 0,000,00 164,65 0,00 658,59 8,00 vdb 0,00 0,000,00 84,85 0,00 339,39 8,00 vdb 0,00 0,000,00 170,71 0,00 682,83 8,00 vdb 0,00 0,000,00 185,71 0,00 742,86 8,00 vdb 0,00 0,000,00 174,75 0,00 698,99 8,00 vdb 0,00 0,000,00 87,88 0,00 351,52 8,00 w/s seems ok After a few moment it would look like this: Device: rrqm/s wrqm/s r/s w/srkB/swkB/s avgrq-sz vdb 0,00 0,000,00 249,00 0,00 996,00 8,00 vdb 0,00 0,000,000,00 0,00 0,00 0,00 vdb 0,00 0,000,00 260,00 0,00 1040,00 8,00 vdb 0,00 0,000,000,00 0,00 0,00 0,00 vdb 0,00 0,000,00 250,00 0,00 1000,00 8,00 vdb 0,00 0,000,00 249,49 0,00 997,98 8,00 vdb 0,00 0,000,000,00 0,00 0,00 0,00 Here w/s start to oscillate on a few second cycle. Waiting around ten hours leads to this. Device: rrqm/s wrqm/s r/s w/srkB/swkB/s avgrq-sz db 0,00 0,000,000,00 0,00 0,00 0,00 vdb 0,00 0,000,00 267,68 0,00 1070,71 8,00 vdb 0,00 0,000,00 1184,38 0,00 4737,50 8,00 vdb 0,00 0,000,000,00 0,00 0,00 0,00 vdb 0,00 0,000,000,00 0,00 0,00 0,00 vdb 0,00 0,000,000,00 0,00 0,00 0,00 vdb 0,00 0,000,000,00 0,00 0,00 0,00 vdb 0,00 0,000,000,00 0,00 0,00 0,00 vdb 0,00 0,000,000,00 0,00 0,00 0,00 vdb 0,00 0,000,000,00 0,00 0,00 0,00 vdb 0,00 0,000,000,00 0,00 0,00 0,00 vdb 0,00 0,000,00 1415,15 0,00 5660,61 8,00 vdb 0,00 0,000,000,00 0,00 0,00 0,00 vdb 0,00 0,000,000,00 0,00 0,00 0,00 vdb 0,00 0,000,000,00 0,00 0,00 0,00 vdb 0,00 0,000,000,00 0,00 0,00 0,00 vdb 0,00 0,000,000,00 0,00 0,00 0,00 vdb 0,00 0,000,000,00 0,00 0,00 0,00 vdb 0,00 0,000,000,00 0,00 0,00 0,00 vdb 0,00 0,000,000,00 0,00 0,00 0,00 vdb 0,00 0,000,00 939,18 0,00 3756,70 8,00 vdb 0,00 0,000,00 523,71 0,00 2094,85 8,00 It seems to get worse with the amplitude and duration of the cycle growing. As the cause of the amplification of the oscillation seems to be the growing of the cycle duration I wrote the "block: fix bdrv_exceed_iops_limits wait computation" patch which solve this problem. load.c - beware of hardcoded "/dev/vdb" --- #define _GNU_SOURCE #include #include #include #include #include #include #define BLOCKSIZE 4096 #define THREAD_COUNT 50 #define IOS 10 pthread_mutex_t mutex; pthread_cond_t condition; void *io_thread_body(void *opaque) { int j, fd; int i = (int) *(int *) opaque; void *buffer; posix_memalign(&buffer, BLOCKSIZE, BLOCKSIZE); fd = open("/dev/vdb", O_RDWR|O_DIRECT); if (fd < 0) { printf("Fail to open /dev/vdb in O_DIRECT\n"); goto exit; } printf("Waiting for signal\n"); pthread_mutex_lock(&mutex); pthread_cond_wait(&condition, &mutex); pthread_mutex_unlock(&mutex); printf("Starting ios\n"); while (1) { off_t offset = (i * IOS * BLOCKSIZE) + j * BLOCKSIZE; pwrite(fd, buffer, BLOCKSIZE, offset); j++; } exit: free(opaque); pthread_exit(NULL); } int main(int argc, char **argv) { int i; pthread_t threads[THREAD_COUNT]; pthread_attr_t attr; pthread_mutex_init(&mutex, NULL); pthread_cond_init(&condition, NULL); pthread_attr_init(&attr); pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_J
[Qemu-devel] [PATCH] block: fix bdrv_exceed_iops_limits wait computation
This patch fix an I/O throttling behavior triggered by limiting at 150 iops and running a load of 50 threads doing random pwrites on a raw virtio device. After a few moments the iops count start to oscillate steadily between 0 and a value upper than the limit. As the load keep running the period and the amplitude of the oscillation increase until io bursts reaching the physical storage max iops count are done. These bursts are followed by guest io starvation. As the period of this oscillation cycle is increasing the cause must be a computation error leading to increase slowly the wait time. This patch make the wait time a bit smaller and tests confirm that it solves the oscillating behavior. Signed-off-by: Benoit Canet --- block.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block.c b/block.c index 0a062c9..455d8b0 100644 --- a/block.c +++ b/block.c @@ -3739,7 +3739,7 @@ static bool bdrv_exceed_iops_limits(BlockDriverState *bs, bool is_write, } /* Calc approx time to dispatch */ -wait_time = (ios_base + 1) / iops_limit; +wait_time = ios_base / iops_limit; if (wait_time > elapsed_time) { wait_time = wait_time - elapsed_time; } else { -- 1.7.10.4
Re: [Qemu-devel] [PATCH] virtio-blk-x: fix configuration synchronization.
On Wed, 20 Mar 2013 10:00:01 +0100 fred.kon...@greensocs.com wrote: > From: KONRAD Frederic > > The virtio-blk-x configuration is not in sync with virtio-blk configuration. > So this patch remove the virtio-blk-x configuration field, and use virtio-blk > one for setting the properties. > > This also remove a useless configuration copy in virtio_blk_device_init. > > Note that this is on top of "virtio-ccw queue as of 2013/03/20". Seems to work on both s390-virtio and virtio-ccw (comes up and I can specify e.g. config-wce). info qtree still shows the properties for both virtio-blk-x and virtio-blk, but they are in sync now. > > Signed-off-by: KONRAD Frederic Tested-by: Cornelia Huck > --- > hw/s390x/s390-virtio-bus.c | 4 ++-- > hw/s390x/s390-virtio-bus.h | 1 - > hw/s390x/virtio-ccw.c | 4 ++-- > hw/s390x/virtio-ccw.h | 1 - > hw/virtio-blk.c| 7 --- > hw/virtio-blk.h| 2 -- > hw/virtio-pci.c| 7 --- > hw/virtio-pci.h| 1 - > 8 files changed, 8 insertions(+), 19 deletions(-) > > diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c > index c5d5456..b6f9682 100644 > --- a/hw/s390x/s390-virtio-bus.c > +++ b/hw/s390x/s390-virtio-bus.c > @@ -166,7 +166,7 @@ static int s390_virtio_blk_init(VirtIOS390Device > *s390_dev) > { > VirtIOBlkS390 *dev = VIRTIO_BLK_S390(s390_dev); > DeviceState *vdev = DEVICE(&dev->vdev); > -virtio_blk_set_conf(vdev, &(dev->blk)); > + > qdev_set_parent_bus(vdev, BUS(&s390_dev->bus)); > if (qdev_init(vdev) < 0) { > return -1; > @@ -435,7 +435,7 @@ static const TypeInfo s390_virtio_net = { > }; > > static Property s390_virtio_blk_properties[] = { > -DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkS390, blk), > +DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkS390, vdev.blk), > DEFINE_PROP_END_OF_LIST(), > }; > > diff --git a/hw/s390x/s390-virtio-bus.h b/hw/s390x/s390-virtio-bus.h > index 1a63411..05347bc 100644 > --- a/hw/s390x/s390-virtio-bus.h > +++ b/hw/s390x/s390-virtio-bus.h > @@ -128,7 +128,6 @@ void s390_virtio_reset_idx(VirtIOS390Device *dev); > typedef struct VirtIOBlkS390 { > VirtIOS390Device parent_obj; > VirtIOBlock vdev; > -VirtIOBlkConf blk; > } VirtIOBlkS390; > > > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c > index 4c44b7e..4a10bd6 100644 > --- a/hw/s390x/virtio-ccw.c > +++ b/hw/s390x/virtio-ccw.c > @@ -574,7 +574,7 @@ static int virtio_ccw_blk_init(VirtioCcwDevice *ccw_dev) > { > VirtIOBlkCcw *dev = VIRTIO_BLK_CCW(ccw_dev); > DeviceState *vdev = DEVICE(&dev->vdev); > -virtio_blk_set_conf(vdev, &(dev->blk)); > + > qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus)); > if (qdev_init(vdev) < 0) { > return -1; > @@ -757,7 +757,7 @@ static const TypeInfo virtio_ccw_net = { > static Property virtio_ccw_blk_properties[] = { > DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id), > DEFINE_VIRTIO_BLK_FEATURES(VirtioCcwDevice, host_features[0]), > -DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkCcw, blk), > +DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOBlkCcw, vdev.blk), > DEFINE_PROP_END_OF_LIST(), > }; > > diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h > index 3993bc5..8f7c3a5 100644 > --- a/hw/s390x/virtio-ccw.h > +++ b/hw/s390x/virtio-ccw.h > @@ -102,7 +102,6 @@ typedef struct VirtualCssBus { > typedef struct VirtIOBlkCcw { > VirtioCcwDevice parent_obj; > VirtIOBlock vdev; > -VirtIOBlkConf blk; > } VirtIOBlkCcw; > > > diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c > index f2143fd..0414a8a 100644 > --- a/hw/virtio-blk.c > +++ b/hw/virtio-blk.c > @@ -619,12 +619,6 @@ static const BlockDevOps virtio_block_ops = { > .resize_cb = virtio_blk_resize, > }; > > -void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk) > -{ > -VirtIOBlock *s = VIRTIO_BLK(dev); > -memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf)); > -} > - > static int virtio_blk_device_init(VirtIODevice *vdev) > { > DeviceState *qdev = DEVICE(vdev); > @@ -656,7 +650,6 @@ static int virtio_blk_device_init(VirtIODevice *vdev) > vdev->reset = virtio_blk_reset; > s->bs = blk->conf.bs; > s->conf = &blk->conf; > -memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf)); > s->rq = NULL; > s->sector_mask = (s->conf->logical_block_size / BDRV_SECTOR_SIZE) - 1; > > diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h > index 8c6c78b..e228057 100644 > --- a/hw/virtio-blk.h > +++ b/hw/virtio-blk.h > @@ -148,6 +148,4 @@ typedef struct VirtIOBlock { > DEFINE_PROP_BIT("config-wce", _state, _field.config_wce, 0, true) > #endif /* __linux__ */ > > -void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk); > - > #endif > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c > index f3ece78..31a78de 100644 > --- a/hw/virtio-pci.c > +++ b/hw/virtio-pci.c > @@ -1405,10 +1405,11 @@ static Property virtio_blk_pci_properties[] = { > VIRTIO_P
Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with panicked event
Hu Tao writes: > pvevent device is used to send guest panic event from guest to qemu. > > When guest panic happens, pvevent device driver will write a event > number to IO port 0x505(which is the IO port occupied by pvevent device, > by default). On receiving the event, pvevent device will pause guest > cpu(s), and send a qmp event QEVENT_GUEST_PANICKED. > > TODO: make the IO port configurable > > Signed-off-by: Wen Congyang > Signed-off-by: Hu Tao > --- > hw/Makefile.objs | 2 + > hw/pvevent.c | 116 > +++ > 2 files changed, 118 insertions(+) > create mode 100644 hw/pvevent.c > > diff --git a/hw/Makefile.objs b/hw/Makefile.objs > index 40ebe46..edf499e 100644 > --- a/hw/Makefile.objs > +++ b/hw/Makefile.objs > @@ -218,5 +218,7 @@ obj-$(CONFIG_KVM) += ivshmem.o > obj-$(CONFIG_LINUX) += vfio_pci.o > endif > > +common-obj-y += pvevent.o > + > $(obj)/baum.o: QEMU_CFLAGS += $(SDL_CFLAGS) > endif > diff --git a/hw/pvevent.c b/hw/pvevent.c > new file mode 100644 > index 000..c7df77b > --- /dev/null > +++ b/hw/pvevent.c > @@ -0,0 +1,116 @@ > +/* > + * QEMU simulated pvevent device. > + * > + * Copyright Fujitsu, Corp. 2013 > + * > + * Authors: > + * Wen Congyang > + * Hu Tao > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + * > + */ > + > +#include > +#include > +#include > +#include > +#include > + > +/* The bit of supported pv event */ > +#define PVEVENT_F_PANICKED 0 > + > +/* The pv event value */ > +#define PVEVENT_PANICKED(1 << PVEVENT_F_PANICKED) This looks like events are encoded as bits, which can be combined into a set, but... > + > +#define TYPE_ISA_PVEVENT_DEVICE"pvevent" > +#define ISA_PVEVENT_DEVICE(obj)\ > +OBJECT_CHECK(PVEventState, (obj), TYPE_ISA_PVEVENT_DEVICE) > + > +static void panicked_mon_event(const char *action) > +{ > +QObject *data; > + > +data = qobject_from_jsonf("{ 'action': %s }", action); > +monitor_protocol_event(QEVENT_GUEST_PANICKED, data); > +qobject_decref(data); > +} > + > +static void handle_event(int event) > +{ > +if (event == PVEVENT_PANICKED) { ... here you don't test bits. Weird. Intentional? > +panicked_mon_event("pause"); > +vm_stop(RUN_STATE_GUEST_PANICKED); > +return; > +} > +} Should we log events we don't understand? > + > +#include "hw/isa.h" > + > +typedef struct PVEventState { > +ISADevice parent_obj; > + > +MemoryRegion io; > +uint16_t ioport; > +} PVEventState; > + > +/* return supported events on read */ > +static uint64_t pvevent_ioport_read(void *opaque, hwaddr addr, unsigned size) > +{ > +return PVEVENT_PANICKED; > +} > + > +static void pvevent_ioport_write(void *opaque, hwaddr addr, uint64_t val, > + unsigned size) > +{ > +handle_event(val); > +} [...]
Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with panicked event
Paolo Bonzini writes: > Il 14/03/2013 16:59, Gleb Natapov ha scritto: [...] >> Fine with me. That was just a suggestion. I thought we had singleton >> qdev flag. > > We have no_user, but it's broken and not exactly a match for what you > want here. For what it's worth, no_user is set for this device.
Re: [Qemu-devel] [PATCH v14 3/4] introduce pvevent device to deal with panicked event
Alexander Graf writes: > On 14.03.2013, at 10:43, Paolo Bonzini wrote: > >> Il 14/03/2013 10:19, Gleb Natapov ha scritto: >>> On Thu, Mar 14, 2013 at 10:14:12AM +0100, Paolo Bonzini wrote: Il 14/03/2013 09:15, Hu Tao ha scritto: > pvevent device is used to send guest panic event from guest to qemu. > > When guest panic happens, pvevent device driver will write a event > number to IO port 0x505(which is the IO port occupied by pvevent device, > by default). On receiving the event, pvevent device will pause guest > cpu(s), and send a qmp event QEVENT_GUEST_PANICKED. > > TODO: make the IO port configurable The port is already configurable as far as the device is concerned; when you add the port to the PC boards you will have to wind up fw-cfg. >>> >>> Why not add fw-cfg when device is created (with -device for instance)? >> >> It depends on what we decide is the supported interface for the device: >> >> * it can be an ISA device; the interface is the I/O port and ACPI > > Is there any particular reason it's an ISA device with a PIO port, > rather than a platform / sysbus device with MMIO access? With the > latter, we could easily reuse the device on other platforms (ppc, arm) > and even the guest driver code for platforms that do ACPI (arm?). As Paolo and Gleb observed, this is the most natural way to do such a device on x86. Other systems may want different connectors, e.g. sysbus/MMIO. We already have devices coming in variants with different connectors, e.g. serial and fdc. They share core code just fine. Some boilerplate gets duplicated, but it's not horrible. There has been talk about modelling such things more nicely in QOM. Opportunity for a QOM expert to tell us more about it :) [...]
Re: [Qemu-devel] [Xen-devel] [PATCHv3] QEMU(upstream): Disable xen's use of O_DIRECT by default as it results in crashes.
Il 20/03/2013 09:33, Alex Bligh ha scritto: > Stefano, > > --On 19 March 2013 15:13:29 + Stefano Stabellini > wrote: > >> Therefore I think that the current change is not safe, but it is pretty >> easy to make it safe. >> You just need to move the call to blk_open from blk_init to blk_connect. >> Actually you can move most of blk_init to blk_connect, you just need to >> keep the 4 xenstore_write_be_int at the end of the function. > > The final of these 4 writes in blk_init is: > >xenstore_write_be_int(&blkdev->xendev, "sectors", > blkdev->file_size / blkdev->file_blk); > > and blkdev->filesize comes from bdrv_getlength(blkdev->bs), and that > requires (I think) bdrv_open to have been called. Can these > 4 writes move safely to blk_connect? Or can we write the sectors > count as (say) 0 here and fix it later in blk_connect? The remainder > look ok. I think so. blkfront reads "sectors" when QEMU moves to XenbusStateConnected, in blkfront_connect. blk_connect is called from xen_be_try_initialise, which moves to XenbusStateConnected on success. So, QEMU's blk_connect will always be called before blkfront's blkfront_connect. Paolo
Re: [Qemu-devel] [PATCH V2 0/3] ARM: Misc ARM big-endian bug fixes
On 03/19/2013 12:30 PM, Peter Maydell wrote: > On 19 March 2013 11:21, Fabien Chouteau wrote: >> v2: >> - I drop the the CPSR.E/SCTLR.E/SCTLR.IE patch. The issue is still too >> complex for me to submit a good patch. >> >> - Fix the GDB byte order for 64bits registers >> >> Fabien Chouteau (3): >> QAPI: Add ARMEB target-type >> Add default config for armeb-softmmu > > Hi Fabien. I'm afraid I don't want to take these two unless they > come attached to an actual cpu and board model that use them. > Otherwise they're really just half-a-feature, I think. > Well these patches are very small, but I think it can help people to get started. I submitted them because I thought it was a regression to not be able to build qemu-system-armeb. Regards, -- Fabien Chouteau
Re: [Qemu-devel] [PATCH v14 0/4] pvevent device to deal with guest panic event
Gleb Natapov writes: > On Thu, Mar 14, 2013 at 04:15:49PM +0800, Hu Tao wrote: >> This series introduces a new simulated device, pvevent, to notify >> qemu when guest panic event happens. >> > Call it something less generic. pvpanic for instance. No objection, just observing that the device can indeed transport arbitrary events[*]. It happens to transport just one at the moment, PVEVENT_F_PANICKED. [*] To be precise: a set of 8 event types carrying no additional information.
[Qemu-devel] [PATCH 03/23] pixman: add qemu_pixman_color()
Helper function to map qemu colors (32bit integer + matching PixelFormat) into pixman_color_t. Signed-off-by: Gerd Hoffmann --- include/ui/qemu-pixman.h |2 ++ ui/qemu-pixman.c | 11 +++ 2 files changed, 13 insertions(+) diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h index b032f52..b0f09b5 100644 --- a/include/ui/qemu-pixman.h +++ b/include/ui/qemu-pixman.h @@ -43,4 +43,6 @@ pixman_image_t *qemu_pixman_mirror_create(pixman_format_code_t format, pixman_image_t *image); void qemu_pixman_image_unref(pixman_image_t *image); +pixman_color_t qemu_pixman_color(PixelFormat *pf, uint32_t color); + #endif /* QEMU_PIXMAN_H */ diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c index 6dcbe90..be551e0 100644 --- a/ui/qemu-pixman.c +++ b/ui/qemu-pixman.c @@ -79,3 +79,14 @@ void qemu_pixman_image_unref(pixman_image_t *image) } pixman_image_unref(image); } + +pixman_color_t qemu_pixman_color(PixelFormat *pf, uint32_t color) +{ +pixman_color_t c; + +c.red = ((color & pf->rmask) >> pf->rshift) << (16 - pf->rbits); +c.green = ((color & pf->gmask) >> pf->gshift) << (16 - pf->gbits); +c.blue = ((color & pf->bmask) >> pf->bshift) << (16 - pf->bbits); +c.alpha = ((color & pf->amask) >> pf->ashift) << (16 - pf->abits); +return c; +} -- 1.7.9.7
[Qemu-devel] [PATCH 04/23] pixman: render vgafont glyphs into pixman images
Add helper functions to create pixman mask images for glyphs and to render these glyphs into a pixman image. Signed-off-by: Gerd Hoffmann --- include/ui/qemu-pixman.h |7 +++ ui/qemu-pixman.c | 43 +++ 2 files changed, 50 insertions(+) diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h index b0f09b5..f012ec5 100644 --- a/include/ui/qemu-pixman.h +++ b/include/ui/qemu-pixman.h @@ -44,5 +44,12 @@ pixman_image_t *qemu_pixman_mirror_create(pixman_format_code_t format, void qemu_pixman_image_unref(pixman_image_t *image); pixman_color_t qemu_pixman_color(PixelFormat *pf, uint32_t color); +pixman_image_t *qemu_pixman_glyph_from_vgafont(int height, const uint8_t *font, + unsigned int ch); +void qemu_pixman_glyph_render(pixman_image_t *glyph, + pixman_image_t *surface, + pixman_color_t *fgcol, + pixman_color_t *bgcol, + int x, int y, int cw, int ch); #endif /* QEMU_PIXMAN_H */ diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c index be551e0..254bd8c 100644 --- a/ui/qemu-pixman.c +++ b/ui/qemu-pixman.c @@ -90,3 +90,46 @@ pixman_color_t qemu_pixman_color(PixelFormat *pf, uint32_t color) c.alpha = ((color & pf->amask) >> pf->ashift) << (16 - pf->abits); return c; } + +pixman_image_t *qemu_pixman_glyph_from_vgafont(int height, const uint8_t *font, + unsigned int ch) +{ +pixman_image_t *glyph; +uint8_t *data; +bool bit; +int x, y; + +glyph = pixman_image_create_bits(PIXMAN_a8, 8, height, + NULL, 0); +data = (uint8_t *)pixman_image_get_data(glyph); + +font += height * ch; +for (y = 0; y < height; y++, font++) { +for (x = 0; x < 8; x++, data++) { +bit = (*font) & (1 << (7-x)); +*data = bit ? 0xff : 0x00; +} +} +return glyph; +} + +void qemu_pixman_glyph_render(pixman_image_t *glyph, + pixman_image_t *surface, + pixman_color_t *fgcol, + pixman_color_t *bgcol, + int x, int y, int cw, int ch) +{ +pixman_image_t *ifg = pixman_image_create_solid_fill(fgcol); +pixman_image_t *ibg = pixman_image_create_solid_fill(bgcol); + +pixman_image_composite(PIXMAN_OP_SRC, ibg, NULL, surface, + 0, 0, 0, 0, + cw * x, ch * y, + cw, ch); +pixman_image_composite(PIXMAN_OP_OVER, ifg, glyph, surface, + 0, 0, 0, 0, + cw * x, ch * y, + cw, ch); +pixman_image_unref(ifg); +pixman_image_unref(ibg); +} -- 1.7.9.7
[Qemu-devel] [PATCH 01/23] exynos4210_fimd.c: fix display resize bug introduced after console revamp
From: Igor Mitsyanko In exynos4210 display update function, we were acquiring DisplaySurface pointer before calling screen resize function, not paying attention that resize procedure can replace current DisplaySurface with newly allocated one. Right thing to do is to initialize DisplaySurface AFTER a call to resize function. Signed-off-by: Igor Mitsyanko Signed-off-by: Gerd Hoffmann --- hw/exynos4210_fimd.c |7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/exynos4210_fimd.c b/hw/exynos4210_fimd.c index bf316c6..333456a 100644 --- a/hw/exynos4210_fimd.c +++ b/hw/exynos4210_fimd.c @@ -1243,7 +1243,7 @@ static void exynos4210_update_resolution(Exynos4210fimdState *s) static void exynos4210_fimd_update(void *opaque) { Exynos4210fimdState *s = (Exynos4210fimdState *)opaque; -DisplaySurface *surface = qemu_console_surface(s->console); +DisplaySurface *surface; Exynos4210fimdWindow *w; int i, line; hwaddr fb_line_addr, inc_size; @@ -1256,11 +1256,12 @@ static void exynos4210_fimd_update(void *opaque) const int global_height = ((s->vidtcon[2] >> FIMD_VIDTCON2_VER_SHIFT) & FIMD_VIDTCON2_SIZE_MASK) + 1; -if (!s || !s->console || !surface_bits_per_pixel(surface) || -!s->enabled) { +if (!s || !s->console || !s->enabled || +surface_bits_per_pixel(qemu_console_surface(s->console)) == 0) { return; } exynos4210_update_resolution(s); +surface = qemu_console_surface(s->console); for (i = 0; i < NUM_OF_WINDOWS; i++) { w = &s->window[i]; -- 1.7.9.7
[Qemu-devel] [PATCH v2 00/23] console: overhaul continued
Hi, Next round of console cleanup patches for review. What is in there? (1) qemu text consoles are rendered using pixman now. (2) Each QemuConsole has its own DisplaySurface now, so we can switch consoles without re-rendering the QemuConsole and update non-active consoles. (3) Based on (2) the screendump code is simplified *alot*. (4) gui refresh timer adaption is fixes and consolidated. Also some cleanups and bugfixes. v2 adds: * bugfixes from Igor. * multihead support. please review, Gerd The following changes since commit 2d62a95766025e6a0a333528278936e2cc8bf978: virtio-blk: cleanup: remove qdev field. (2013-03-18 13:08:41 -0500) are available in the git repository at: git://git.kraxel.org/qemu pixman.v9 for you to fetch changes up to 48c022d8df21652373d0b20e78bcb13a7fb8f5ea: qxl: register QemuConsole for secondary cards (2013-03-20 10:24:52 +0100) Gerd Hoffmann (21): pixman: add qemu_pixman_color() pixman: render vgafont glyphs into pixman images console: use pixman for fill+blit console: use pixman for font rendering console: switch color_table_rgb to pixman_color_t console: add trace events console: displaystate init revamp console: rename vga_hw_*, add QemuConsole param console: give each QemuConsole its own DisplaySurface console: simplify screendump console: zap g_width + g_height console: move gui_update+gui_setup_refresh from vl.c into console.c console: make DisplayState private to console.c console: add GraphicHwOps console: gui timer fixes xen: re-enable refresh interval reporting for xenfb console: add qemu_console_is_* console: allow pinning displaychangelisteners to consoles gtk: custom cursor support gtk: show a window for each graphical QemuConsole qxl: register QemuConsole for secondary cards Igor Mitsyanko (2): exynos4210_fimd.c: fix display resize bug introduced after console revamp hw/vmware_vga.c: fix screen resize bug introduced after console revamp hw/arm/musicpal.c |8 +- hw/blizzard.c | 21 +- hw/cirrus_vga.c| 10 +- hw/exynos4210_fimd.c | 15 +- hw/g364fb.c| 80 +- hw/jazz_led.c | 11 +- hw/milkymist-vgafb.c |9 +- hw/omap_lcdc.c | 93 +- hw/pl110.c |9 +- hw/pxa2xx_lcd.c|9 +- hw/qxl.c | 42 +-- hw/sm501.c |7 +- hw/ssd0303.c |9 +- hw/ssd0323.c |9 +- hw/tc6393xb.c | 10 +- hw/tcx.c | 143 +- hw/unicore32/puv3.c|4 +- hw/vga-isa-mm.c|4 +- hw/vga-isa.c |3 +- hw/vga-pci.c |3 +- hw/vga.c | 76 + hw/vga_int.h |6 +- hw/vmware_vga.c| 49 +--- hw/xenfb.c | 58 ++-- include/ui/console.h | 55 ++-- include/ui/qemu-pixman.h |9 + include/ui/spice-display.h |1 - trace-events |4 + ui/console.c | 672 ui/curses.c| 11 +- ui/gtk.c | 66 - ui/qemu-pixman.c | 54 ui/sdl.c | 52 ++-- ui/spice-display.c | 11 +- ui/vnc.c | 87 ++ ui/vnc.h |2 - vl.c | 55 +--- 37 files changed, 737 insertions(+), 1030 deletions(-)
[Qemu-devel] [PATCH 08/23] console: add trace events
Signed-off-by: Gerd Hoffmann --- trace-events |3 +++ ui/console.c |4 2 files changed, 7 insertions(+) diff --git a/trace-events b/trace-events index 406fe5f..c241985 100644 --- a/trace-events +++ b/trace-events @@ -958,6 +958,9 @@ dma_bdrv_cb(void *dbs, int ret) "dbs=%p ret=%d" dma_map_wait(void *dbs) "dbs=%p" # console.h +console_gfx_new(void) "" +console_txt_new(int w, int h) "%dx%d" +console_select(int nr) "%d" displaysurface_create(void *display_surface, int w, int h) "surface=%p, %dx%d" displaysurface_create_from(void *display_surface, int w, int h, int bpp, int swap) "surface=%p, %dx%d, bpp %d, bswap %d" displaysurface_free(void *display_surface) "surface=%p" diff --git a/ui/console.c b/ui/console.c index fbec6cb..45fa580 100644 --- a/ui/console.c +++ b/ui/console.c @@ -904,6 +904,8 @@ void console_select(unsigned int index) if (index >= MAX_CONSOLES) return; + +trace_console_select(index); if (active_console) { surface = qemu_console_surface(active_console); active_console->g_width = surface_width(surface); @@ -1367,6 +1369,7 @@ QemuConsole *graphic_console_init(vga_hw_update_ptr update, DisplayState *ds; ds = (DisplayState *) g_malloc0(sizeof(DisplayState)); +trace_console_gfx_new(); s = new_console(ds, GRAPHIC_CONSOLE); s->hw_update = update; s->hw_invalidate = invalidate; @@ -1485,6 +1488,7 @@ static CharDriverState *text_console_init(ChardevVC *vc) height = vc->rows * FONT_HEIGHT; } +trace_console_txt_new(width, height); if (width == 0 || height == 0) { s = new_console(NULL, TEXT_CONSOLE); } else { -- 1.7.9.7
[Qemu-devel] [PATCH 11/23] console: give each QemuConsole its own DisplaySurface
Go away from the global DisplaySurface, give one to each QemuConsole instead. With this patch applied it is possible to call graphics_hw_* functions with qemu consoles which are not the current foreground console. Signed-off-by: Gerd Hoffmann --- include/ui/console.h |1 - ui/console.c | 96 -- 2 files changed, 69 insertions(+), 28 deletions(-) diff --git a/include/ui/console.h b/include/ui/console.h index 9c585c0..0dd66fd 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -183,7 +183,6 @@ struct DisplayChangeListener { }; struct DisplayState { -struct DisplaySurface *surface; struct QEMUTimer *gui_timer; bool have_gfx; bool have_text; diff --git a/ui/console.c b/ui/console.c index 23eed6f..7687ebc 100644 --- a/ui/console.c +++ b/ui/console.c @@ -116,6 +116,7 @@ struct QemuConsole { int index; console_type_t console_type; DisplayState *ds; +DisplaySurface *surface; /* Graphic console state. */ graphic_hw_update_ptr hw_update; @@ -164,6 +165,8 @@ static QemuConsole *consoles[MAX_CONSOLES]; static int nb_consoles = 0; static void text_console_do_init(CharDriverState *chr, DisplayState *ds); +static void dpy_gfx_switch_surface(DisplayState *ds, + DisplaySurface *surface); void graphic_hw_update(QemuConsole *con) { @@ -933,8 +936,9 @@ void console_select(unsigned int index) } active_console = s; if (ds->have_gfx) { -surface = qemu_create_displaysurface(s->g_width, s->g_height); -dpy_gfx_replace_surface(s, surface); +dpy_gfx_switch_surface(ds, s->surface); +dpy_gfx_update(s, 0, 0, surface_width(s->surface), + surface_height(s->surface)); } if (ds->have_text) { dpy_text_resize(s, s->width, s->height); @@ -943,7 +947,6 @@ void console_select(unsigned int index) qemu_mod_timer(s->cursor_timer, qemu_get_clock_ms(rt_clock) + CONSOLE_CURSOR_PERIOD / 2); } -graphic_hw_invalidate(s); } } @@ -1195,8 +1198,8 @@ void register_displaychangelistener(DisplayState *ds, dcl->ds = ds; QLIST_INSERT_HEAD(&ds->listeners, dcl, next); gui_setup_refresh(ds); -if (dcl->ops->dpy_gfx_switch) { -dcl->ops->dpy_gfx_switch(dcl, ds->surface); +if (dcl->ops->dpy_gfx_switch && active_console) { +dcl->ops->dpy_gfx_switch(dcl, active_console->surface); } } @@ -1212,8 +1215,8 @@ void dpy_gfx_update(QemuConsole *con, int x, int y, int w, int h) { DisplayState *s = con->ds; struct DisplayChangeListener *dcl; -int width = pixman_image_get_width(s->surface->image); -int height = pixman_image_get_height(s->surface->image); +int width = surface_width(con->surface); +int height = surface_height(con->surface); x = MAX(x, 0); y = MAX(y, 0); @@ -1222,6 +1225,9 @@ void dpy_gfx_update(QemuConsole *con, int x, int y, int w, int h) w = MIN(w, width - x); h = MIN(h, height - y); +if (con != active_console) { +return; +} QLIST_FOREACH(dcl, &s->listeners, next) { if (dcl->ops->dpy_gfx_update) { dcl->ops->dpy_gfx_update(dcl, x, y, w, h); @@ -1229,19 +1235,28 @@ void dpy_gfx_update(QemuConsole *con, int x, int y, int w, int h) } } -void dpy_gfx_replace_surface(QemuConsole *con, - DisplaySurface *surface) +static void dpy_gfx_switch_surface(DisplayState *ds, + DisplaySurface *surface) { -DisplayState *s = con->ds; -DisplaySurface *old_surface = s->surface; struct DisplayChangeListener *dcl; -s->surface = surface; -QLIST_FOREACH(dcl, &s->listeners, next) { +QLIST_FOREACH(dcl, &ds->listeners, next) { if (dcl->ops->dpy_gfx_switch) { dcl->ops->dpy_gfx_switch(dcl, surface); } } +} + +void dpy_gfx_replace_surface(QemuConsole *con, + DisplaySurface *surface) +{ +DisplayState *s = con->ds; +DisplaySurface *old_surface = con->surface; + +con->surface = surface; +if (con == active_console) { +dpy_gfx_switch_surface(s, surface); +} qemu_free_displaysurface(old_surface); } @@ -1260,6 +1275,10 @@ void dpy_gfx_copy(QemuConsole *con, int src_x, int src_y, { DisplayState *s = con->ds; struct DisplayChangeListener *dcl; + +if (con != active_console) { +return; +} QLIST_FOREACH(dcl, &s->listeners, next) { if (dcl->ops->dpy_gfx_copy) { dcl->ops->dpy_gfx_copy(dcl, src_x, src_y, dst_x, dst_y, w, h); @@ -1273,6 +1292,10 @@ void dpy_text_cursor(QemuConsole *con, int x, int y) { DisplayState *s = con->ds; struct DisplayChangeListener *dcl; + +if (con != active_console) { +return; +} QLIST_FOREACH(dcl, &s->list
[Qemu-devel] [PATCH 09/23] console: displaystate init revamp
We have only one DisplayState, so there is no need for the "next" linking, rip it. Also consolidate all displaystate initialization into init_displaystate(). This function is called by vl.c after creating the devices (and thus all QemuConsoles) and before initializing DisplayChangeListensers (aka gtk/sdl/vnc/spice ui). Signed-off-by: Gerd Hoffmann --- include/ui/console.h |5 +--- ui/console.c | 73 +++--- vl.c |6 + 3 files changed, 36 insertions(+), 48 deletions(-) diff --git a/include/ui/console.h b/include/ui/console.h index a234c72..3725dae 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -189,12 +189,9 @@ struct DisplayState { bool have_text; QLIST_HEAD(, DisplayChangeListener) listeners; - -struct DisplayState *next; }; -void register_displaystate(DisplayState *ds); -DisplayState *get_displaystate(void); +DisplayState *init_displaystate(void); DisplaySurface* qemu_create_displaysurface_from(int width, int height, int bpp, int linesize, uint8_t *data, bool byteswap); diff --git a/ui/console.c b/ui/console.c index 45fa580..2c8fd91 100644 --- a/ui/console.c +++ b/ui/console.c @@ -163,6 +163,8 @@ static QemuConsole *active_console; static QemuConsole *consoles[MAX_CONSOLES]; static int nb_consoles = 0; +static void text_console_do_init(CharDriverState *chr, DisplayState *ds); + void vga_hw_update(void) { if (active_console && active_console->hw_update) @@ -1323,39 +1325,37 @@ bool dpy_cursor_define_supported(QemuConsole *con) return false; } -static void dumb_display_init(void) -{ -DisplayState *ds = g_malloc0(sizeof(DisplayState)); -int width = 640; -int height = 480; - -if (is_fixedsize_console()) { -width = active_console->g_width; -height = active_console->g_height; -} -ds->surface = qemu_create_displaysurface(width, height); - -register_displaystate(ds); -} - /***/ /* register display */ -void register_displaystate(DisplayState *ds) +/* console.c internal use only */ +static DisplayState *get_alloc_displaystate(void) { -DisplayState **s; -s = &display_state; -while (*s != NULL) -s = &(*s)->next; -ds->next = NULL; -*s = ds; +if (!display_state) { +display_state = g_new0(DisplayState, 1); +} +return display_state; } -DisplayState *get_displaystate(void) +/* + * Called by main(), after creating QemuConsoles + * and before initializing ui (sdl/vnc/...). + */ +DisplayState *init_displaystate(void) { +int i; + if (!display_state) { -dumb_display_init (); +display_state = g_new0(DisplayState, 1); } + +for (i = 0; i < nb_consoles; i++) { +if (consoles[i]->console_type != GRAPHIC_CONSOLE && +consoles[i]->ds == NULL) { +text_console_do_init(consoles[i]->chr, display_state); +} +} + return display_state; } @@ -1365,10 +1365,12 @@ QemuConsole *graphic_console_init(vga_hw_update_ptr update, vga_hw_text_update_ptr text_update, void *opaque) { +int width = 640; +int height = 480; QemuConsole *s; DisplayState *ds; -ds = (DisplayState *) g_malloc0(sizeof(DisplayState)); +ds = get_alloc_displaystate(); trace_console_gfx_new(); s = new_console(ds, GRAPHIC_CONSOLE); s->hw_update = update; @@ -1377,9 +1379,9 @@ QemuConsole *graphic_console_init(vga_hw_update_ptr update, s->hw_text_update = text_update; s->hw = opaque; -ds->surface = qemu_create_displaysurface(640, 480); - -register_displaystate(ds); +if (!ds->surface) { +ds->surface = qemu_create_displaysurface(width, height); +} return s; } @@ -1505,6 +1507,10 @@ static CharDriverState *text_console_init(ChardevVC *vc) s->g_height = height; chr->opaque = s; chr->chr_set_echo = text_console_set_echo; + +if (display_state) { +text_console_do_init(chr, display_state); +} return chr; } @@ -1520,17 +1526,6 @@ void register_vc_handler(VcHandler *handler) vc_handler = handler; } -void text_consoles_set_display(DisplayState *ds) -{ -int i; - -for (i = 0; i < nb_consoles; i++) { -if (consoles[i]->console_type != GRAPHIC_CONSOLE) { -text_console_do_init(consoles[i]->chr, ds); -} -} -} - void qemu_console_resize(QemuConsole *s, int width, int height) { s->g_width = width; diff --git a/vl.c b/vl.c index ce51e65..c5eb9a7 100644 --- a/vl.c +++ b/vl.c @@ -4300,8 +4300,7 @@ int main(int argc, char **argv, char **envp) net_check_clients(); -/* just use the first displaystate for the moment */ -ds = get_displaystate(); +ds = init_displaystate
[Qemu-devel] [PATCH 05/23] console: use pixman for fill+blit
Zap homegrown pixel shuffeling code, use pixman calls instead. Signed-off-by: Gerd Hoffmann --- ui/console.c | 65 +- 1 file changed, 10 insertions(+), 55 deletions(-) diff --git a/ui/console.c b/ui/console.c index eb7a2bc..0ec0911 100644 --- a/ui/console.c +++ b/ui/console.c @@ -213,36 +213,14 @@ static void vga_fill_rect(QemuConsole *con, uint32_t color) { DisplaySurface *surface = qemu_console_surface(con); -uint8_t *d, *d1; -int x, y, bpp; +pixman_rectangle16_t rect = { +.x = posx, .y = posy, .width = width, .height = height +}; +pixman_color_t pcolor; -bpp = surface_bytes_per_pixel(surface); -d1 = surface_data(surface) + -surface_stride(surface) * posy + bpp * posx; -for (y = 0; y < height; y++) { -d = d1; -switch(bpp) { -case 1: -for (x = 0; x < width; x++) { -*((uint8_t *)d) = color; -d++; -} -break; -case 2: -for (x = 0; x < width; x++) { -*((uint16_t *)d) = color; -d += 2; -} -break; -case 4: -for (x = 0; x < width; x++) { -*((uint32_t *)d) = color; -d += 4; -} -break; -} -d1 += surface_stride(surface); -} +pcolor = qemu_pixman_color(&surface->pf, color); +pixman_image_fill_rectangles(PIXMAN_OP_SRC, surface->image, + &pcolor, 1, &rect); } /* copy from (xs, ys) to (xd, yd) a rectangle of size (w, h) */ @@ -250,33 +228,10 @@ static void vga_bitblt(QemuConsole *con, int xs, int ys, int xd, int yd, int w, int h) { DisplaySurface *surface = qemu_console_surface(con); -const uint8_t *s; -uint8_t *d; -int wb, y, bpp; -bpp = surface_bytes_per_pixel(surface); -wb = w * bpp; -if (yd <= ys) { -s = surface_data(surface) + -surface_stride(surface) * ys + bpp * xs; -d = surface_data(surface) + -surface_stride(surface) * yd + bpp * xd; -for (y = 0; y < h; y++) { -memmove(d, s, wb); -d += surface_stride(surface); -s += surface_stride(surface); -} -} else { -s = surface_data(surface) + -surface_stride(surface) * (ys + h - 1) + bpp * xs; -d = surface_data(surface) + -surface_stride(surface) * (yd + h - 1) + bpp * xd; - for (y = 0; y < h; y++) { -memmove(d, s, wb); -d -= surface_stride(surface); -s -= surface_stride(surface); -} -} +pixman_image_composite(PIXMAN_OP_SRC, + surface->image, NULL, surface->image, + xs, ys, 0, 0, xd, yd, w, h); } /***/ -- 1.7.9.7
[Qemu-devel] [PATCH 06/23] console: use pixman for font rendering
Zap homegrown font rendering code, use pixman calls instead. Signed-off-by: Gerd Hoffmann --- ui/console.c | 110 ++ 1 file changed, 11 insertions(+), 99 deletions(-) diff --git a/ui/console.c b/ui/console.c index 0ec0911..4e79268 100644 --- a/ui/console.c +++ b/ui/console.c @@ -242,45 +242,6 @@ static void vga_bitblt(QemuConsole *con, #include "vgafont.h" -#define cbswap_32(__x) \ -((uint32_t)( \ - (((uint32_t)(__x) & (uint32_t)0x00ffUL) << 24) | \ - (((uint32_t)(__x) & (uint32_t)0xff00UL) << 8) | \ - (((uint32_t)(__x) & (uint32_t)0x00ffUL) >> 8) | \ - (((uint32_t)(__x) & (uint32_t)0xff00UL) >> 24) )) - -#ifdef HOST_WORDS_BIGENDIAN -#define PAT(x) x -#else -#define PAT(x) cbswap_32(x) -#endif - -static const uint32_t dmask16[16] = { -PAT(0x), -PAT(0x00ff), -PAT(0xff00), -PAT(0x), -PAT(0x00ff), -PAT(0x00ff00ff), -PAT(0x0000), -PAT(0x00ff), -PAT(0xff00), -PAT(0xffff), -PAT(0xff00ff00), -PAT(0xff00), -PAT(0x), -PAT(0x00ff), -PAT(0xff00), -PAT(0x), -}; - -static const uint32_t dmask4[4] = { -PAT(0x), -PAT(0x), -PAT(0x), -PAT(0x), -}; - #ifndef CONFIG_CURSES enum color_names { COLOR_BLACK = 0, @@ -353,17 +314,11 @@ static void console_print_text_attributes(TextAttributes *t_attrib, char ch) static void vga_putcharxy(QemuConsole *s, int x, int y, int ch, TextAttributes *t_attrib) { +static pixman_image_t *glyphs[256]; DisplaySurface *surface = qemu_console_surface(s); -uint8_t *d; -const uint8_t *font_ptr; -unsigned int font_data, linesize, xorcol, bpp; -int i; unsigned int fgcol, bgcol; - -#ifdef DEBUG_CONSOLE -printf("x: %2i y: %2i", x, y); -console_print_text_attributes(t_attrib, ch); -#endif +pixman_image_t *ifg, *ibg; +pixman_color_t cfg, cbg; if (t_attrib->invers) { bgcol = color_table_rgb[t_attrib->bold][t_attrib->fgcol]; @@ -372,59 +327,16 @@ static void vga_putcharxy(QemuConsole *s, int x, int y, int ch, fgcol = color_table_rgb[t_attrib->bold][t_attrib->fgcol]; bgcol = color_table_rgb[t_attrib->bold][t_attrib->bgcol]; } +cfg = qemu_pixman_color(&surface->pf, fgcol); +cbg = qemu_pixman_color(&surface->pf, bgcol); +ifg = pixman_image_create_solid_fill(&cfg); +ibg = pixman_image_create_solid_fill(&cbg); -bpp = surface_bytes_per_pixel(surface); -d = surface_data(surface) + -surface_stride(surface) * y * FONT_HEIGHT + bpp * x * FONT_WIDTH; -linesize = surface_stride(surface); -font_ptr = vgafont16 + FONT_HEIGHT * ch; -xorcol = bgcol ^ fgcol; -switch (surface_bits_per_pixel(surface)) { -case 8: -for(i = 0; i < FONT_HEIGHT; i++) { -font_data = *font_ptr++; -if (t_attrib->uline -&& ((i == FONT_HEIGHT - 2) || (i == FONT_HEIGHT - 3))) { -font_data = 0xFF; -} -((uint32_t *)d)[0] = (dmask16[(font_data >> 4)] & xorcol) ^ bgcol; -((uint32_t *)d)[1] = (dmask16[(font_data >> 0) & 0xf] & xorcol) ^ bgcol; -d += linesize; -} -break; -case 16: -case 15: -for(i = 0; i < FONT_HEIGHT; i++) { -font_data = *font_ptr++; -if (t_attrib->uline -&& ((i == FONT_HEIGHT - 2) || (i == FONT_HEIGHT - 3))) { -font_data = 0xFF; -} -((uint32_t *)d)[0] = (dmask4[(font_data >> 6)] & xorcol) ^ bgcol; -((uint32_t *)d)[1] = (dmask4[(font_data >> 4) & 3] & xorcol) ^ bgcol; -((uint32_t *)d)[2] = (dmask4[(font_data >> 2) & 3] & xorcol) ^ bgcol; -((uint32_t *)d)[3] = (dmask4[(font_data >> 0) & 3] & xorcol) ^ bgcol; -d += linesize; -} -break; -case 32: -for(i = 0; i < FONT_HEIGHT; i++) { -font_data = *font_ptr++; -if (t_attrib->uline && ((i == FONT_HEIGHT - 2) || (i == FONT_HEIGHT - 3))) { -font_data = 0xFF; -} -((uint32_t *)d)[0] = (-((font_data >> 7)) & xorcol) ^ bgcol; -((uint32_t *)d)[1] = (-((font_data >> 6) & 1) & xorcol) ^ bgcol; -((uint32_t *)d)[2] = (-((font_data >> 5) & 1) & xorcol) ^ bgcol; -((uint32_t *)d)[3] = (-((font_data >> 4) & 1) & xorcol) ^ bgcol; -((uint32_t *)d)[4] = (-((font_data >> 3) & 1) & xorcol) ^ bgcol; -((uint32_t *)d)[5] = (-((font_data >> 2) & 1) & xorcol) ^ bgcol; -((uint32_t *)d)[6] = (-((font_data >> 1) & 1) & xorcol) ^ bgcol; -((uint32_t *)d)[7] = (-((font_data >> 0) & 1) & xorcol) ^ bgcol; -d += linesize; -} -break; +if (!glyphs[ch]) { +g
[Qemu-devel] [PATCH 21/23] gtk: custom cursor support
Makes gtk ui play nicely with qxl (and vmware_svga) as you can actually see your pointer now ;) Signed-off-by: Gerd Hoffmann --- ui/gtk.c | 25 + 1 file changed, 25 insertions(+) diff --git a/ui/gtk.c b/ui/gtk.c index 7599ff4..512e974 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -303,6 +303,29 @@ static void gd_refresh(DisplayChangeListener *dcl) graphic_hw_update(dcl->con); } +static void gd_mouse_set(DisplayChangeListener *dcl, + int x, int y, int visible) +{ +/* should warp pointer to x, y here */ +} + +static void gd_cursor_define(DisplayChangeListener *dcl, + QEMUCursor *c) +{ +GtkDisplayState *s = container_of(dcl, GtkDisplayState, dcl); +GdkPixbuf *pixbuf; +GdkCursor *cursor; + +pixbuf = gdk_pixbuf_new_from_data((guchar *)(c->data), + GDK_COLORSPACE_RGB, true, 8, + c->width, c->height, c->width * 4, + NULL, NULL); +cursor = gdk_cursor_new_from_pixbuf(gdk_display_get_default(), +pixbuf, c->hot_x, c->hot_y); +gdk_window_set_cursor(s->drawing_area->window, cursor); +g_object_unref(pixbuf); +} + static void gd_switch(DisplayChangeListener *dcl, DisplaySurface *surface) { @@ -1309,6 +1332,8 @@ static const DisplayChangeListenerOps dcl_ops = { .dpy_gfx_update= gd_update, .dpy_gfx_switch= gd_switch, .dpy_refresh = gd_refresh, +.dpy_mouse_set = gd_mouse_set, +.dpy_cursor_define = gd_cursor_define, }; void gtk_display_init(DisplayState *ds) -- 1.7.9.7
[Qemu-devel] [PATCH 22/23] gtk: show a window for each graphical QemuConsole
Multihead support: For each graphical console we'll create a gtk window, so with multiple graphics cards installed you get a gtk window for each. vte tabs are attached to the console #0 window. --- ui/gtk.c | 40 +++- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/ui/gtk.c b/ui/gtk.c index 512e974..1c86054 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -159,8 +159,6 @@ typedef struct GtkDisplayState bool external_pause_update; } GtkDisplayState; -static GtkDisplayState *global_state; - /** Utility Functions **/ static bool gd_is_grab_active(GtkDisplayState *s) @@ -1210,7 +1208,7 @@ static void gd_connect_signals(GtkDisplayState *s) G_CALLBACK(gd_leave_event), s); } -static void gd_create_menus(GtkDisplayState *s) +static void gd_create_menus(GtkDisplayState *s, bool vtetabs) { GtkStockItem item; GtkAccelGroup *accel_group; @@ -1302,11 +1300,13 @@ static void gd_create_menus(GtkDisplayState *s) gtk_accel_map_add_entry("/View/VGA", GDK_KEY_1, GDK_CONTROL_MASK | GDK_MOD1_MASK); gtk_menu_shell_append(GTK_MENU_SHELL(s->view_menu), s->vga_item); -for (i = 0; i < nb_vcs; i++) { -VirtualConsole *vc = &s->vc[i]; +if (vtetabs) { +for (i = 0; i < nb_vcs; i++) { +VirtualConsole *vc = &s->vc[i]; -group = gd_vc_init(s, vc, i, group); -s->nb_vcs++; +group = gd_vc_init(s, vc, i, group); +s->nb_vcs++; +} } separator = gtk_separator_menu_item_new(); @@ -1336,14 +1336,13 @@ static const DisplayChangeListenerOps dcl_ops = { .dpy_cursor_define = gd_cursor_define, }; -void gtk_display_init(DisplayState *ds) +static void gtk_display_init_one(DisplayState *ds, QemuConsole *con, + bool vtetabs) { GtkDisplayState *s = g_malloc0(sizeof(*s)); -gtk_init(NULL, NULL); - s->dcl.ops = &dcl_ops; -s->dcl.con = qemu_console_lookup_by_index(0); +s->dcl.con = con; s->window = gtk_window_new(GTK_WINDOW_TOPLEVEL); #if GTK_CHECK_VERSION(3, 2, 0) @@ -1371,7 +1370,7 @@ void gtk_display_init(DisplayState *ds) gtk_notebook_append_page(GTK_NOTEBOOK(s->notebook), s->drawing_area, gtk_label_new("VGA")); -gd_create_menus(s); +gd_create_menus(s, vtetabs); gd_connect_signals(s); @@ -1400,6 +1399,21 @@ void gtk_display_init(DisplayState *ds) gtk_widget_show_all(s->window); register_displaychangelistener(ds, &s->dcl); +} -global_state = s; +void gtk_display_init(DisplayState *ds) +{ +QemuConsole *con; +int i = 0; + +gtk_init(NULL, NULL); + +con = qemu_console_lookup_by_index(i++); +gtk_display_init_one(ds, con, true); +while ((con = qemu_console_lookup_by_index(i++)) != NULL) { +if (!qemu_console_is_graphic(con)) { +break; +} +gtk_display_init_one(ds, con, false); +} } -- 1.7.9.7
[Qemu-devel] [PATCH 02/23] hw/vmware_vga.c: fix screen resize bug introduced after console revamp
From: Igor Mitsyanko In vmsvga display update function, a pointer to DisplaySurface must be acquired after a call to vmsvga_check_size since this function might replace current DisplaySurface with a new one. Signed-off-by: Igor Mitsyanko Signed-off-by: Gerd Hoffmann --- hw/vmware_vga.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c index 5b9ce8f..c0aac31 100644 --- a/hw/vmware_vga.c +++ b/hw/vmware_vga.c @@ -995,7 +995,7 @@ static inline void vmsvga_check_size(struct vmsvga_state_s *s) static void vmsvga_update_display(void *opaque) { struct vmsvga_state_s *s = opaque; -DisplaySurface *surface = qemu_console_surface(s->vga.con); +DisplaySurface *surface; bool dirty = false; if (!s->enable) { @@ -1004,6 +1004,7 @@ static void vmsvga_update_display(void *opaque) } vmsvga_check_size(s); +surface = qemu_console_surface(s->vga.con); vmsvga_fifo_run(s); vmsvga_update_rect_flush(s); -- 1.7.9.7
[Qemu-devel] [PATCH 13/23] console: zap g_width + g_height
We have a surface per QemuConsole now, so there is no need to keep track of the QemuConsole size any more as we can query the surface size directly at any time. Signed-off-by: Gerd Hoffmann --- ui/console.c | 32 +--- 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/ui/console.c b/ui/console.c index 241720b..771f155 100644 --- a/ui/console.c +++ b/ui/console.c @@ -123,7 +123,6 @@ struct QemuConsole { graphic_hw_invalidate_ptr hw_invalidate; graphic_hw_text_update_ptr hw_text_update; void *hw; -int g_width, g_height; /* Text console state */ int width; @@ -384,8 +383,8 @@ static void text_console_resize(QemuConsole *s) int w1, x, y, last_width; last_width = s->width; -s->width = s->g_width / FONT_WIDTH; -s->height = s->g_height / FONT_HEIGHT; +s->width = surface_width(s->surface) / FONT_WIDTH; +s->height = surface_height(s->surface) / FONT_HEIGHT; w1 = last_width; if (s->width < w1) @@ -946,18 +945,12 @@ static void console_putchar(QemuConsole *s, int ch) void console_select(unsigned int index) { -DisplaySurface *surface; QemuConsole *s; if (index >= MAX_CONSOLES) return; trace_console_select(index); -if (active_console) { -surface = qemu_console_surface(active_console); -active_console->g_width = surface_width(surface); -active_console->g_height = surface_height(surface); -} s = consoles[index]; if (s) { DisplayState *ds = s->ds; @@ -1084,11 +1077,8 @@ void kbd_put_keysym(int keysym) static void text_console_invalidate(void *opaque) { QemuConsole *s = (QemuConsole *) opaque; -DisplaySurface *surface = qemu_console_surface(s); if (s->ds->have_text && s->console_type == TEXT_CONSOLE) { -s->g_width = surface_width(surface); -s->g_height = surface_height(surface); text_console_resize(s); } console_refresh(s); @@ -1492,6 +1482,8 @@ static void text_console_update_cursor(void *opaque) static void text_console_do_init(CharDriverState *chr, DisplayState *ds) { QemuConsole *s; +int g_width = 80 * FONT_WIDTH; +int g_height = 24 * FONT_HEIGHT; s = chr->opaque; @@ -1507,16 +1499,13 @@ static void text_console_do_init(CharDriverState *chr, DisplayState *ds) s->total_height = DEFAULT_BACKSCROLL; s->x = 0; s->y = 0; -if (s->console_type == TEXT_CONSOLE) { +if (!s->surface) { if (active_console && active_console->surface) { -s->g_width = surface_width(active_console->surface); -s->g_height = surface_height(active_console->surface); -} else { -s->g_width = 80 * FONT_WIDTH; -s->g_height = 24 * FONT_HEIGHT; +g_width = surface_width(active_console->surface); +g_height = surface_height(active_console->surface); } +s->surface = qemu_create_displaysurface(g_width, g_height); } -s->surface = qemu_create_displaysurface(s->g_width, s->g_height); s->cursor_timer = qemu_new_timer_ms(rt_clock, text_console_update_cursor, s); @@ -1578,6 +1567,7 @@ static CharDriverState *text_console_init(ChardevVC *vc) s = new_console(NULL, TEXT_CONSOLE); } else { s = new_console(NULL, TEXT_CONSOLE_FIXED_SIZE); +s->surface = qemu_create_displaysurface(width, height); } if (!s) { @@ -1586,8 +1576,6 @@ static CharDriverState *text_console_init(ChardevVC *vc) } s->chr = chr; -s->g_width = width; -s->g_height = height; chr->opaque = s; chr->chr_set_echo = text_console_set_echo; @@ -1614,8 +1602,6 @@ void qemu_console_resize(QemuConsole *s, int width, int height) DisplaySurface *surface; assert(s->console_type == GRAPHIC_CONSOLE); -s->g_width = width; -s->g_height = height; surface = qemu_create_displaysurface(width, height); dpy_gfx_replace_surface(s, surface); } -- 1.7.9.7
[Qemu-devel] [PATCH 12/23] console: simplify screendump
Screendumps are alot simpler as we can update non-active QemuConsoles now. So we only need to update the QemuConsole we want write out, then dump the DisplaySurface content into a ppm file. Done. No console switching needed. No special support code in the gfx card emulation needed. Zap it all. Also move ppm_save out of the vga code and next to the qmp_screendump function. For now screen dumping is limited to console #0 (like it used to be), even though it is dead simple to extend it to other consoles. I wanna finish the console cleanup before setting new qapi interfaces into stone. --- hw/arm/musicpal.c|2 +- hw/blizzard.c| 14 +- hw/cirrus_vga.c |4 +- hw/exynos4210_fimd.c |2 +- hw/g364fb.c | 73 +--- hw/jazz_led.c|1 - hw/milkymist-vgafb.c |2 +- hw/omap_lcdc.c | 86 + hw/pl110.c |2 +- hw/pxa2xx_lcd.c |2 +- hw/qxl.c | 22 + hw/sm501.c |2 +- hw/ssd0303.c |2 +- hw/ssd0323.c |2 +- hw/tc6393xb.c|1 - hw/tcx.c | 129 +- hw/unicore32/puv3.c |2 +- hw/vga-isa-mm.c |2 +- hw/vga-isa.c |2 +- hw/vga-pci.c |2 +- hw/vga.c | 66 -- hw/vga_int.h |2 - hw/vmware_vga.c | 26 -- hw/xenfb.c |1 - include/ui/console.h |3 -- ui/console.c | 69 +++ 26 files changed, 69 insertions(+), 452 deletions(-) diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c index edd5282..4e9a23c 100644 --- a/hw/arm/musicpal.c +++ b/hw/arm/musicpal.c @@ -612,7 +612,7 @@ static int musicpal_lcd_init(SysBusDevice *dev) sysbus_init_mmio(dev, &s->iomem); s->con = graphic_console_init(lcd_refresh, lcd_invalidate, - NULL, NULL, s); + NULL, s); qemu_console_resize(s->con, 128*3, 64*3); qdev_init_gpio_in(&dev->qdev, musicpal_lcd_gpio_brigthness_in, 3); diff --git a/hw/blizzard.c b/hw/blizzard.c index 020d3de..891bff8 100644 --- a/hw/blizzard.c +++ b/hw/blizzard.c @@ -933,18 +933,6 @@ static void blizzard_update_display(void *opaque) s->my[1] = 0; } -static void blizzard_screen_dump(void *opaque, const char *filename, - bool cswitch, Error **errp) -{ -BlizzardState *s = (BlizzardState *) opaque; -DisplaySurface *surface = qemu_console_surface(s->con); - -blizzard_update_display(opaque); -if (s && surface_data(surface)) { -ppm_save(filename, surface, errp); -} -} - #define DEPTH 8 #include "hw/blizzard_template.h" #define DEPTH 15 @@ -965,7 +953,7 @@ void *s1d13745_init(qemu_irq gpio_int) s->con = graphic_console_init(blizzard_update_display, blizzard_invalidate_display, - blizzard_screen_dump, NULL, s); + NULL, s); surface = qemu_console_surface(s->con); switch (surface_bits_per_pixel(surface)) { diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c index 8a0f74f..03b3245 100644 --- a/hw/cirrus_vga.c +++ b/hw/cirrus_vga.c @@ -2911,7 +2911,7 @@ static int vga_initfn(ISADevice *dev) cirrus_init_common(&d->cirrus_vga, CIRRUS_ID_CLGD5430, 0, isa_address_space(dev), isa_address_space_io(dev)); s->con = graphic_console_init(s->update, s->invalidate, - s->screen_dump, s->text_update, + s->text_update, s); rom_add_vga(VGABIOS_CIRRUS_FILENAME); /* XXX ISA-LFB support */ @@ -2960,7 +2960,7 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) cirrus_init_common(s, device_id, 1, pci_address_space(dev), pci_address_space_io(dev)); s->vga.con = graphic_console_init(s->vga.update, s->vga.invalidate, - s->vga.screen_dump, s->vga.text_update, + s->vga.text_update, &s->vga); /* setup PCI */ diff --git a/hw/exynos4210_fimd.c b/hw/exynos4210_fimd.c index 333456a..8178917 100644 --- a/hw/exynos4210_fimd.c +++ b/hw/exynos4210_fimd.c @@ -1902,7 +1902,7 @@ static int exynos4210_fimd_init(SysBusDevice *dev) "exynos4210.fimd", FIMD_REGS_SIZE); sysbus_init_mmio(dev, &s->iomem); s->console = graphic_console_init(exynos4210_fimd_update, - exynos4210_fimd_invalidate, NULL, NULL, s); + exynos4210_fimd_invalidate, NULL, s); return 0; } diff --git a/hw/g364fb.c b/hw/g364fb.c index f7014e9..b70fe8a 100644 --- a/hw/g364fb.c +++ b/hw/g364fb.c @@ -294,77 +294,6 @@ static void g36
[Qemu-devel] [PATCH 16/23] console: add GraphicHwOps
Pass a single GraphicHwOps struct pointer to graphic_console_init, instead of a bunch of function pointers. Signed-off-by: Gerd Hoffmann --- hw/arm/musicpal.c|8 ++-- hw/blizzard.c|9 ++--- hw/cirrus_vga.c |8 ++-- hw/exynos4210_fimd.c |8 ++-- hw/g364fb.c |9 ++--- hw/jazz_led.c| 10 +++--- hw/milkymist-vgafb.c |9 ++--- hw/omap_lcdc.c |9 ++--- hw/pl110.c |9 ++--- hw/pxa2xx_lcd.c |9 ++--- hw/qxl.c | 16 ++-- hw/sm501.c |7 +-- hw/ssd0303.c |9 ++--- hw/ssd0323.c |9 ++--- hw/tc6393xb.c|9 + hw/tcx.c | 18 -- hw/unicore32/puv3.c |4 +++- hw/vga-isa-mm.c |4 +--- hw/vga-isa.c |3 +-- hw/vga-pci.c |3 +-- hw/vga.c | 10 +++--- hw/vga_int.h |4 +--- hw/vmware_vga.c | 20 hw/xenfb.c | 10 ++ include/ui/console.h | 12 ++-- ui/console.c | 32 +++- 26 files changed, 154 insertions(+), 104 deletions(-) diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c index 4e9a23c..7d3e239 100644 --- a/hw/arm/musicpal.c +++ b/hw/arm/musicpal.c @@ -601,6 +601,11 @@ static const MemoryRegionOps musicpal_lcd_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; +static const GraphicHwOps musicpal_gfx_ops = { +.invalidate = lcd_invalidate, +.gfx_update = lcd_refresh, +}; + static int musicpal_lcd_init(SysBusDevice *dev) { musicpal_lcd_state *s = FROM_SYSBUS(musicpal_lcd_state, dev); @@ -611,8 +616,7 @@ static int musicpal_lcd_init(SysBusDevice *dev) "musicpal-lcd", MP_LCD_SIZE); sysbus_init_mmio(dev, &s->iomem); -s->con = graphic_console_init(lcd_refresh, lcd_invalidate, - NULL, s); +s->con = graphic_console_init(&musicpal_gfx_ops, s); qemu_console_resize(s->con, 128*3, 64*3); qdev_init_gpio_in(&dev->qdev, musicpal_lcd_gpio_brigthness_in, 3); diff --git a/hw/blizzard.c b/hw/blizzard.c index 891bff8..794b276 100644 --- a/hw/blizzard.c +++ b/hw/blizzard.c @@ -944,6 +944,11 @@ static void blizzard_update_display(void *opaque) #define DEPTH 32 #include "hw/blizzard_template.h" +static const GraphicHwOps blizzard_ops = { +.invalidate = blizzard_invalidate_display, +.gfx_update = blizzard_update_display, +}; + void *s1d13745_init(qemu_irq gpio_int) { BlizzardState *s = (BlizzardState *) g_malloc0(sizeof(*s)); @@ -951,9 +956,7 @@ void *s1d13745_init(qemu_irq gpio_int) s->fb = g_malloc(0x18); -s->con = graphic_console_init(blizzard_update_display, - blizzard_invalidate_display, - NULL, s); +s->con = graphic_console_init(&blizzard_ops, s); surface = qemu_console_surface(s->con); switch (surface_bits_per_pixel(surface)) { diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c index 03b3245..ed7962f 100644 --- a/hw/cirrus_vga.c +++ b/hw/cirrus_vga.c @@ -2910,9 +2910,7 @@ static int vga_initfn(ISADevice *dev) vga_common_init(s); cirrus_init_common(&d->cirrus_vga, CIRRUS_ID_CLGD5430, 0, isa_address_space(dev), isa_address_space_io(dev)); -s->con = graphic_console_init(s->update, s->invalidate, - s->text_update, - s); +s->con = graphic_console_init(s->hw_ops, s); rom_add_vga(VGABIOS_CIRRUS_FILENAME); /* XXX ISA-LFB support */ /* FIXME not qdev yet */ @@ -2959,9 +2957,7 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) vga_common_init(&s->vga); cirrus_init_common(s, device_id, 1, pci_address_space(dev), pci_address_space_io(dev)); - s->vga.con = graphic_console_init(s->vga.update, s->vga.invalidate, - s->vga.text_update, - &s->vga); + s->vga.con = graphic_console_init(s->vga.hw_ops, &s->vga); /* setup PCI */ diff --git a/hw/exynos4210_fimd.c b/hw/exynos4210_fimd.c index 8178917..b92922a 100644 --- a/hw/exynos4210_fimd.c +++ b/hw/exynos4210_fimd.c @@ -1888,6 +1888,11 @@ static const VMStateDescription exynos4210_fimd_vmstate = { } }; +static const GraphicHwOps exynos4210_fimd_ops = { +.invalidate = exynos4210_fimd_invalidate, +.gfx_update = exynos4210_fimd_update, +}; + static int exynos4210_fimd_init(SysBusDevice *dev) { Exynos4210fimdState *s = FROM_SYSBUS(Exynos4210fimdState, dev); @@ -1901,8 +1906,7 @@ static int exynos4210_fimd_init(SysBusDevice *dev) memory_region_init_io(&s->iomem, &exynos4210_fimd_mmio_ops, s, "exynos4210.fimd", FIMD_REGS_SIZE); sysbus_init_mmio(dev, &s->iomem); -s->console = graphic_c
[Qemu-devel] [PATCH 15/23] console: make DisplayState private to console.c
With gui_* being moved to console.c nobody outside console.c needs access to DisplayState fields any more. Make the struct private. Signed-off-by: Gerd Hoffmann --- include/ui/console.h |8 ui/console.c |8 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/include/ui/console.h b/include/ui/console.h index d92626b..50cd7b0 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -182,14 +182,6 @@ struct DisplayChangeListener { QLIST_ENTRY(DisplayChangeListener) next; }; -struct DisplayState { -struct QEMUTimer *gui_timer; -bool have_gfx; -bool have_text; - -QLIST_HEAD(, DisplayChangeListener) listeners; -}; - DisplayState *init_displaystate(void); DisplaySurface* qemu_create_displaysurface_from(int width, int height, int bpp, int linesize, uint8_t *data, diff --git a/ui/console.c b/ui/console.c index 8e8f918..c22895f 100644 --- a/ui/console.c +++ b/ui/console.c @@ -157,6 +157,14 @@ struct QemuConsole { QEMUTimer *kbd_timer; }; +struct DisplayState { +struct QEMUTimer *gui_timer; +bool have_gfx; +bool have_text; + +QLIST_HEAD(, DisplayChangeListener) listeners; +}; + static DisplayState *display_state; static QemuConsole *active_console; static QemuConsole *consoles[MAX_CONSOLES]; -- 1.7.9.7
[Qemu-devel] [PATCH 18/23] xen: re-enable refresh interval reporting for xenfb
xenfb informs the guest about the gui refresh interval so it can avoid pointless work. That logic was temporarely disabled for the DisplayState reorganization. Restore it now, with a proper interface for it. Signed-off-by: Gerd Hoffmann --- hw/xenfb.c | 47 --- include/ui/console.h |1 + ui/console.c |6 ++ 3 files changed, 23 insertions(+), 31 deletions(-) diff --git a/hw/xenfb.c b/hw/xenfb.c index f2af7eb..6344f11 100644 --- a/hw/xenfb.c +++ b/hw/xenfb.c @@ -646,7 +646,7 @@ static void xenfb_guest_copy(struct XenFB *xenfb, int x, int y, int w, int h) dpy_gfx_update(xenfb->c.con, x, y, w, h); } -#if 0 /* def XENFB_TYPE_REFRESH_PERIOD */ +#ifdef XENFB_TYPE_REFRESH_PERIOD static int xenfb_queue_full(struct XenFB *xenfb) { struct xenfb_page *page = xenfb->c.page; @@ -705,37 +705,7 @@ static void xenfb_update(void *opaque) return; if (xenfb->feature_update) { -#if 0 /* XENFB_TYPE_REFRESH_PERIOD */ -struct DisplayChangeListener *l; -int period = ; -int idle = 1; - - if (xenfb_queue_full(xenfb)) - return; - -QLIST_FOREACH(l, &xenfb->c.ds->listeners, next) { -if (l->idle) -continue; -idle = 0; -if (!l->gui_timer_interval) { -if (period > GUI_REFRESH_INTERVAL) -period = GUI_REFRESH_INTERVAL; -} else { -if (period > l->gui_timer_interval) -period = l->gui_timer_interval; -} -} -if (idle) - period = XENFB_NO_REFRESH; - - if (xenfb->refresh_period != period) { - xenfb_send_refresh_period(xenfb, period); - xenfb->refresh_period = period; -xen_be_printf(&xenfb->c.xendev, 1, "refresh period: %d\n", period); - } -#else ; /* nothing */ -#endif } else { /* we don't get update notifications, thus use the * sledge hammer approach ... */ @@ -785,6 +755,20 @@ static void xenfb_update(void *opaque) xenfb->up_fullscreen = 0; } +static void xenfb_update_interval(void *opaque, uint64_t interval) +{ +struct XenFB *xenfb = opaque; + +if (xenfb->feature_update) { +#ifdef XENFB_TYPE_REFRESH_PERIOD +if (xenfb_queue_full(xenfb)) { +return; +} +xenfb_send_refresh_period(xenfb, interval); +#endif +} +} + /* QEMU display state changed, so refresh the framebuffer copy */ static void xenfb_invalidate(void *opaque) { @@ -980,6 +964,7 @@ struct XenDevOps xen_framebuffer_ops = { static const GraphicHwOps xenfb_ops = { .invalidate = xenfb_invalidate, .gfx_update = xenfb_update, +.update_interval = xenfb_update_interval, }; /* diff --git a/include/ui/console.h b/include/ui/console.h index 3cb0018..800f458 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -272,6 +272,7 @@ typedef struct GraphicHwOps { void (*invalidate)(void *opaque); void (*gfx_update)(void *opaque); void (*text_update)(void *opaque, console_ch_t *text); +void (*update_interval)(void *opaque, uint64_t interval); } GraphicHwOps; QemuConsole *graphic_console_init(const GraphicHwOps *ops, diff --git a/ui/console.c b/ui/console.c index 7c496e9..51f9fac 100644 --- a/ui/console.c +++ b/ui/console.c @@ -182,6 +182,7 @@ static void gui_update(void *opaque) uint64_t dcl_interval; DisplayState *ds = opaque; DisplayChangeListener *dcl; +int i; ds->refreshing = true; dpy_refresh(ds); @@ -196,6 +197,11 @@ static void gui_update(void *opaque) } if (ds->update_interval != interval) { ds->update_interval = interval; +for (i = 0; i < nb_consoles; i++) { +if (consoles[i]->hw_ops->update_interval) { +consoles[i]->hw_ops->update_interval(consoles[i]->hw, interval); +} +} trace_console_refresh(interval); } ds->last_update = qemu_get_clock_ms(rt_clock); -- 1.7.9.7
[Qemu-devel] [PATCH 0/5] for spice post load char device hook
This reworks my former patch (http://patchwork.ozlabs.org/patch/227678/ - sorry Hans, can't find the version you posted) per Gerd's suggestion. Specifically it adds a new qemu_chr_fe_post_load api that is called by the front end and implemented by the backend. virtio-console implements it, adding it's own hook which is called by virtio-serial-bus upon post_load from the timer, so that qemu_chr_fe_post_load is called when the vm is already in the running state. This makes the spice-qemu-char usage very simple by not requiring yet another timer. Alon Levy (5): char: add a post_load callback merge to char.h virtio-serial: add a post_load callback implemented by port virtio-console: implement post_load to call to qemu_chr_fe_post_load spice-qemu-char: register interface on post load hw/virtio-console.c| 11 +++ hw/virtio-serial-bus.c | 5 + hw/virtio-serial.h | 2 ++ include/char/char.h| 9 + qemu-char.c| 7 +++ spice-qemu-char.c | 9 + 6 files changed, 43 insertions(+) -- 1.8.1.4
[Qemu-devel] [PATCH 10/23] console: rename vga_hw_*, add QemuConsole param
Add QemuConsole parameter to vga_hw_*, so the interface allows to update non-active consoles (the actual code can't handle this yet, see next patch). Passing NULL is allowed and updates the active console, like the functions do today. While touching all vga_hw_* calls anyway rename that to the functions to hardware-neutral graphics_hw_* Signed-off-by: Gerd Hoffmann --- hw/cirrus_vga.c |2 +- hw/qxl.c |2 +- hw/vga.c |2 +- hw/vga_int.h |8 include/ui/console.h | 22 +++--- ui/console.c | 49 ++--- ui/curses.c |4 ++-- ui/gtk.c |2 +- ui/sdl.c | 18 +- ui/spice-display.c |2 +- ui/vnc.c | 12 ++-- 11 files changed, 67 insertions(+), 56 deletions(-) diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c index 7a4d634..8a0f74f 100644 --- a/hw/cirrus_vga.c +++ b/hw/cirrus_vga.c @@ -720,7 +720,7 @@ static void cirrus_do_copy(CirrusVGAState *s, int dst, int src, int w, int h) /* we have to flush all pending changes so that the copy is generated at the appropriate moment in time */ if (notify) - vga_hw_update(); +graphic_hw_update(s->vga.con); (*s->cirrus_rop) (s, s->vga.vram_ptr + (s->cirrus_blt_dstaddr & s->cirrus_addr_mask), diff --git a/hw/qxl.c b/hw/qxl.c index b66b414..1ceee7e 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -1074,7 +1074,7 @@ static void qxl_enter_vga_mode(PCIQXLDevice *d) qemu_spice_create_host_primary(&d->ssd); d->mode = QXL_MODE_VGA; vga_dirty_log_start(&d->vga); -vga_hw_update(); +graphic_hw_update(d->vga.con); } static void qxl_exit_vga_mode(PCIQXLDevice *d) diff --git a/hw/vga.c b/hw/vga.c index 59bfb22..533b60e 100644 --- a/hw/vga.c +++ b/hw/vga.c @@ -2452,6 +2452,6 @@ static void vga_screen_dump(void *opaque, const char *filename, bool cswitch, if (cswitch) { vga_invalidate_display(s); } -vga_hw_update(); +graphic_hw_update(s->con); ppm_save(filename, surface, errp); } diff --git a/hw/vga_int.h b/hw/vga_int.h index 260f7d6..1b8f670 100644 --- a/hw/vga_int.h +++ b/hw/vga_int.h @@ -152,10 +152,10 @@ typedef struct VGACommonState { uint32_t cursor_offset; unsigned int (*rgb_to_pixel)(unsigned int r, unsigned int g, unsigned b); -vga_hw_update_ptr update; -vga_hw_invalidate_ptr invalidate; -vga_hw_screen_dump_ptr screen_dump; -vga_hw_text_update_ptr text_update; +graphic_hw_update_ptr update; +graphic_hw_invalidate_ptr invalidate; +graphic_hw_screen_dump_ptr screen_dump; +graphic_hw_text_update_ptr text_update; bool full_update_text; bool full_update_gfx; /* hardware mouse cursor support */ diff --git a/include/ui/console.h b/include/ui/console.h index 3725dae..9c585c0 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -278,21 +278,21 @@ static inline void console_write_ch(console_ch_t *dest, uint32_t ch) *dest = ch; } -typedef void (*vga_hw_update_ptr)(void *); -typedef void (*vga_hw_invalidate_ptr)(void *); -typedef void (*vga_hw_screen_dump_ptr)(void *, const char *, bool cswitch, +typedef void (*graphic_hw_update_ptr)(void *); +typedef void (*graphic_hw_invalidate_ptr)(void *); +typedef void (*graphic_hw_screen_dump_ptr)(void *, const char *, bool cswitch, Error **errp); -typedef void (*vga_hw_text_update_ptr)(void *, console_ch_t *); +typedef void (*graphic_hw_text_update_ptr)(void *, console_ch_t *); -QemuConsole *graphic_console_init(vga_hw_update_ptr update, - vga_hw_invalidate_ptr invalidate, - vga_hw_screen_dump_ptr screen_dump, - vga_hw_text_update_ptr text_update, +QemuConsole *graphic_console_init(graphic_hw_update_ptr update, + graphic_hw_invalidate_ptr invalidate, + graphic_hw_screen_dump_ptr screen_dump, + graphic_hw_text_update_ptr text_update, void *opaque); -void vga_hw_update(void); -void vga_hw_invalidate(void); -void vga_hw_text_update(console_ch_t *chardata); +void graphic_hw_update(QemuConsole *con); +void graphic_hw_invalidate(QemuConsole *con); +void graphic_hw_text_update(QemuConsole *con, console_ch_t *chardata); int is_graphic_console(void); int is_fixedsize_console(void); diff --git a/ui/console.c b/ui/console.c index 2c8fd91..23eed6f 100644 --- a/ui/console.c +++ b/ui/console.c @@ -118,10 +118,10 @@ struct QemuConsole { DisplayState *ds; /* Graphic console state. */ -vga_hw_update_ptr hw_update; -vga_hw_invalidate_ptr hw_invalidate; -vga_hw_screen_dump_ptr hw_screen_dump; -vga_hw_text_update_ptr hw_text_update; +
[Qemu-devel] [PATCH 07/23] console: switch color_table_rgb to pixman_color_t
Now that all text console rendering uses pixman we can easily switch the color tables to use pixman_color_t directly. Signed-off-by: Gerd Hoffmann --- ui/console.c | 24 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/ui/console.c b/ui/console.c index 4e79268..fbec6cb 100644 --- a/ui/console.c +++ b/ui/console.c @@ -32,9 +32,6 @@ #define MAX_CONSOLES 12 #define CONSOLE_CURSOR_PERIOD 500 -#define QEMU_RGBA(r, g, b, a) (((a) << 24) | ((r) << 16) | ((g) << 8) | (b)) -#define QEMU_RGB(r, g, b) QEMU_RGBA(r, g, b, 0xff) - typedef struct TextAttributes { uint8_t fgcol:4; uint8_t bgcol:4; @@ -210,17 +207,15 @@ void vga_hw_text_update(console_ch_t *chardata) static void vga_fill_rect(QemuConsole *con, int posx, int posy, int width, int height, - uint32_t color) + pixman_color_t color) { DisplaySurface *surface = qemu_console_surface(con); pixman_rectangle16_t rect = { .x = posx, .y = posy, .width = width, .height = height }; -pixman_color_t pcolor; -pcolor = qemu_pixman_color(&surface->pf, color); pixman_image_fill_rectangles(PIXMAN_OP_SRC, surface->image, - &pcolor, 1, &rect); + &color, 1, &rect); } /* copy from (xs, ys) to (xd, yd) a rectangle of size (w, h) */ @@ -255,7 +250,10 @@ enum color_names { }; #endif -static const uint32_t color_table_rgb[2][8] = { +#define QEMU_RGB(r, g, b) \ +{ .red = r << 8, .green = g << 8, .blue = b << 8, .alpha = 0 } + +static const pixman_color_t color_table_rgb[2][8] = { { /* dark */ QEMU_RGB(0x00, 0x00, 0x00), /* black */ QEMU_RGB(0xaa, 0x00, 0x00), /* red */ @@ -316,9 +314,7 @@ static void vga_putcharxy(QemuConsole *s, int x, int y, int ch, { static pixman_image_t *glyphs[256]; DisplaySurface *surface = qemu_console_surface(s); -unsigned int fgcol, bgcol; -pixman_image_t *ifg, *ibg; -pixman_color_t cfg, cbg; +pixman_color_t fgcol, bgcol; if (t_attrib->invers) { bgcol = color_table_rgb[t_attrib->bold][t_attrib->fgcol]; @@ -327,16 +323,12 @@ static void vga_putcharxy(QemuConsole *s, int x, int y, int ch, fgcol = color_table_rgb[t_attrib->bold][t_attrib->fgcol]; bgcol = color_table_rgb[t_attrib->bold][t_attrib->bgcol]; } -cfg = qemu_pixman_color(&surface->pf, fgcol); -cbg = qemu_pixman_color(&surface->pf, bgcol); -ifg = pixman_image_create_solid_fill(&cfg); -ibg = pixman_image_create_solid_fill(&cbg); if (!glyphs[ch]) { glyphs[ch] = qemu_pixman_glyph_from_vgafont(FONT_HEIGHT, vgafont16, ch); } qemu_pixman_glyph_render(glyphs[ch], surface->image, - &cfg, &cbg, x, y, FONT_WIDTH, FONT_HEIGHT); + &fgcol, &bgcol, x, y, FONT_WIDTH, FONT_HEIGHT); } static void text_console_resize(QemuConsole *s) -- 1.7.9.7
[Qemu-devel] [PATCH 23/23] qxl: register QemuConsole for secondary cards
Hook secondary qxl cards properly into the qemu console subsystem. Watch multihead in action: qemu -display gtk -vga qxl -device qxl -usbdevice tablet Signed-off-by: Gerd Hoffmann --- hw/qxl.c |6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hw/qxl.c b/hw/qxl.c index 320c017..e484610 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -1769,7 +1769,10 @@ static void qxl_hw_invalidate(void *opaque) PCIQXLDevice *qxl = opaque; VGACommonState *vga = &qxl->vga; -vga->hw_ops->invalidate(vga); +if (qxl->mode == QXL_MODE_VGA) { +vga->hw_ops->invalidate(vga); +return; +} } static void qxl_hw_text_update(void *opaque, console_ch_t *chardata) @@ -2085,6 +2088,7 @@ static int qxl_init_secondary(PCIDevice *dev) memory_region_init_ram(&qxl->vga.vram, "qxl.vgavram", qxl->vga.vram_size); vmstate_register_ram(&qxl->vga.vram, &qxl->pci.qdev); qxl->vga.vram_ptr = memory_region_get_ram_ptr(&qxl->vga.vram); +qxl->vga.con = graphic_console_init(&qxl_ops, qxl); return qxl_init_common(qxl); } -- 1.7.9.7
[Qemu-devel] [PATCH 17/23] console: gui timer fixes
Make gui update rate adaption code in gui_update() actually work. Sprinkle in a tracepoint so you can see the code at work. Remove the update rate adaption code in vnc and make vnc simply use the generic bits instead. Signed-off-by: Gerd Hoffmann --- include/ui/console.h |9 --- trace-events |1 + ui/console.c | 34 ui/sdl.c | 10 +++ ui/vnc.c | 71 ++ ui/vnc.h |2 -- 6 files changed, 60 insertions(+), 67 deletions(-) diff --git a/include/ui/console.h b/include/ui/console.h index f3e7791..3cb0018 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -21,7 +21,8 @@ #define QEMU_CAPS_LOCK_LED (1 << 2) /* in ms */ -#define GUI_REFRESH_INTERVAL 30 +#define GUI_REFRESH_INTERVAL_DEFAULT30 +#define GUI_REFRESH_INTERVAL_IDLE 3000 typedef void QEMUPutKBDEvent(void *opaque, int keycode); typedef void QEMUPutLEDEvent(void *opaque, int ledstate); @@ -174,8 +175,7 @@ typedef struct DisplayChangeListenerOps { } DisplayChangeListenerOps; struct DisplayChangeListener { -int idle; -uint64_t gui_timer_interval; +uint64_t update_interval; const DisplayChangeListenerOps *ops; DisplayState *ds; @@ -207,12 +207,13 @@ static inline int is_buffer_shared(DisplaySurface *surface) void register_displaychangelistener(DisplayState *ds, DisplayChangeListener *dcl); +void update_displaychangelistener(DisplayChangeListener *dcl, + uint64_t interval); void unregister_displaychangelistener(DisplayChangeListener *dcl); void dpy_gfx_update(QemuConsole *con, int x, int y, int w, int h); void dpy_gfx_replace_surface(QemuConsole *con, DisplaySurface *surface); -void dpy_refresh(DisplayState *s); void dpy_gfx_copy(QemuConsole *con, int src_x, int src_y, int dst_x, int dst_y, int w, int h); void dpy_text_cursor(QemuConsole *con, int x, int y); diff --git a/trace-events b/trace-events index c241985..88b1070 100644 --- a/trace-events +++ b/trace-events @@ -961,6 +961,7 @@ dma_map_wait(void *dbs) "dbs=%p" console_gfx_new(void) "" console_txt_new(int w, int h) "%dx%d" console_select(int nr) "%d" +console_refresh(int interval) "interval %d ms" displaysurface_create(void *display_surface, int w, int h) "surface=%p, %dx%d" displaysurface_create_from(void *display_surface, int w, int h, int bpp, int swap) "surface=%p, %dx%d, bpp %d, bswap %d" displaysurface_free(void *display_surface) "surface=%p" diff --git a/ui/console.c b/ui/console.c index e85ecf1..7c496e9 100644 --- a/ui/console.c +++ b/ui/console.c @@ -157,6 +157,9 @@ struct QemuConsole { struct DisplayState { struct QEMUTimer *gui_timer; +uint64_t last_update; +uint64_t update_interval; +bool refreshing; bool have_gfx; bool have_text; @@ -171,22 +174,32 @@ static int nb_consoles = 0; static void text_console_do_init(CharDriverState *chr, DisplayState *ds); static void dpy_gfx_switch_surface(DisplayState *ds, DisplaySurface *surface); +static void dpy_refresh(DisplayState *s); static void gui_update(void *opaque) { -uint64_t interval = GUI_REFRESH_INTERVAL; +uint64_t interval = GUI_REFRESH_INTERVAL_IDLE; +uint64_t dcl_interval; DisplayState *ds = opaque; DisplayChangeListener *dcl; +ds->refreshing = true; dpy_refresh(ds); +ds->refreshing = false; QLIST_FOREACH(dcl, &ds->listeners, next) { -if (dcl->gui_timer_interval && -dcl->gui_timer_interval < interval) { -interval = dcl->gui_timer_interval; +dcl_interval = dcl->update_interval ? +dcl->update_interval : GUI_REFRESH_INTERVAL_DEFAULT; +if (interval > dcl_interval) { +interval = dcl_interval; } } -qemu_mod_timer(ds->gui_timer, interval + qemu_get_clock_ms(rt_clock)); +if (ds->update_interval != interval) { +ds->update_interval = interval; +trace_console_refresh(interval); +} +ds->last_update = qemu_get_clock_ms(rt_clock); +qemu_mod_timer(ds->gui_timer, ds->last_update + interval); } static void gui_setup_refresh(DisplayState *ds) @@ -1280,6 +1293,17 @@ void register_displaychangelistener(DisplayState *ds, } } +void update_displaychangelistener(DisplayChangeListener *dcl, + uint64_t interval) +{ +DisplayState *ds = dcl->ds; + +dcl->update_interval = interval; +if (!ds->refreshing && ds->update_interval > interval) { +qemu_mod_timer(ds->gui_timer, ds->last_update + interval); +} +} + void unregister_displaychangelistener(DisplayChangeListener *dcl) { DisplayState *ds = dcl->ds; diff --git a/ui/sdl.c b/ui/sdl.c index ede31dc..97764a6 100644 --- a/ui/sdl.c +++ b/ui/sdl.c @@ -751,12 +751,12 @@ static
[Qemu-devel] [PATCH v2 0/4] for spice post load char device hook
This reworks my former patch (http://patchwork.ozlabs.org/patch/227678/ - sorry Hans, can't find the version you posted) per Gerd's suggestion. Specifically it adds a new qemu_chr_fe_post_load api that is called by the front end and implemented by the backend. virtio-console implements it, adding it's own hook which is called by virtio-serial-bus upon post_load from the timer, so that qemu_chr_fe_post_load is called when the vm is already in the running state. This makes the spice-qemu-char usage very simple by not requiring yet another timer. Note about the added api: I decided to pass "connected" via qemu_chr_fe_post_load in order not to introduce more api on qemu_chr_fe_.. for state querying, like qemu_chr_fe_is_connected, since I'm not sure it would make sense for other frontends. v1 wasn't completely sent to the list, mistakenly sent before squashing one patch. Alon Levy (4): char: add a post_load callback virtio-serial: add a post_load callback implemented by port virtio-console: implement post_load to call to qemu_chr_fe_post_load spice-qemu-char: register interface on post load hw/virtio-console.c| 11 +++ hw/virtio-serial-bus.c | 5 + hw/virtio-serial.h | 2 ++ include/char/char.h| 12 qemu-char.c| 7 +++ spice-qemu-char.c | 9 + 6 files changed, 46 insertions(+) -- 1.8.1.4
[Qemu-devel] [PATCH 19/23] console: add qemu_console_is_*
Signed-off-by: Gerd Hoffmann --- include/ui/console.h |6 +++-- ui/console.c | 59 -- ui/curses.c |7 +++--- ui/sdl.c | 24 ++-- ui/vnc.c |6 ++--- 5 files changed, 56 insertions(+), 46 deletions(-) diff --git a/include/ui/console.h b/include/ui/console.h index 800f458..bcd0139 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -282,8 +282,10 @@ void graphic_hw_update(QemuConsole *con); void graphic_hw_invalidate(QemuConsole *con); void graphic_hw_text_update(QemuConsole *con, console_ch_t *chardata); -int is_graphic_console(void); -int is_fixedsize_console(void); +bool qemu_console_is_visible(QemuConsole *con); +bool qemu_console_is_graphic(QemuConsole *con); +bool qemu_console_is_fixedsize(QemuConsole *con); + void text_consoles_set_display(DisplayState *ds); void console_select(unsigned int index); void console_color_init(DisplayState *ds); diff --git a/ui/console.c b/ui/console.c index 51f9fac..0f2fea5 100644 --- a/ui/console.c +++ b/ui/console.c @@ -509,7 +509,7 @@ static void update_xy(QemuConsole *s, int x, int y) TextCell *c; int y1, y2; -if (s != active_console) { +if (!qemu_console_is_visible(s)) { return; } @@ -537,7 +537,7 @@ static void console_show_cursor(QemuConsole *s, int show) int y, y1; int x = s->x; -if (s != active_console) { +if (!qemu_console_is_visible(s)) { return; } @@ -573,8 +573,9 @@ static void console_refresh(QemuConsole *s) TextCell *c; int x, y, y1; -if (s != active_console) +if (!qemu_console_is_visible(s)) { return; +} if (s->ds->have_text) { s->text_x[0] = 0; @@ -605,15 +606,10 @@ static void console_refresh(QemuConsole *s) } } -static void console_scroll(int ydelta) +static void console_scroll(QemuConsole *s, int ydelta) { -QemuConsole *s; int i, y1; -s = active_console; -if (!s || (s->console_type == GRAPHIC_CONSOLE)) -return; - if (ydelta > 0) { for(i = 0; i < ydelta; i++) { if (s->y_displayed == s->y_base) @@ -663,7 +659,7 @@ static void console_put_lf(QemuConsole *s) c->t_attrib = s->t_attrib_default; c++; } -if (s == active_console && s->y_displayed == s->y_base) { +if (qemu_console_is_visible(s) && s->y_displayed == s->y_base) { if (s->ds->have_text) { s->text_x[0] = 0; s->text_y[0] = 0; @@ -1106,16 +1102,16 @@ void kbd_put_keysym(int keysym) switch(keysym) { case QEMU_KEY_CTRL_UP: -console_scroll(-1); +console_scroll(s, -1); break; case QEMU_KEY_CTRL_DOWN: -console_scroll(1); +console_scroll(s, 1); break; case QEMU_KEY_CTRL_PAGEUP: -console_scroll(-10); +console_scroll(s, -10); break; case QEMU_KEY_CTRL_PAGEDOWN: -console_scroll(10); +console_scroll(s, 10); break; default: /* convert the QEMU keysym to VT100 key string */ @@ -1332,7 +1328,7 @@ void dpy_gfx_update(QemuConsole *con, int x, int y, int w, int h) w = MIN(w, width - x); h = MIN(h, height - y); -if (con != active_console) { +if (!qemu_console_is_visible(con)) { return; } QLIST_FOREACH(dcl, &s->listeners, next) { @@ -1361,7 +1357,7 @@ void dpy_gfx_replace_surface(QemuConsole *con, DisplaySurface *old_surface = con->surface; con->surface = surface; -if (con == active_console) { +if (qemu_console_is_visible(con)) { dpy_gfx_switch_surface(s, surface); } qemu_free_displaysurface(old_surface); @@ -1383,7 +1379,7 @@ void dpy_gfx_copy(QemuConsole *con, int src_x, int src_y, DisplayState *s = con->ds; struct DisplayChangeListener *dcl; -if (con != active_console) { +if (!qemu_console_is_visible(con)) { return; } QLIST_FOREACH(dcl, &s->listeners, next) { @@ -1400,7 +1396,7 @@ void dpy_text_cursor(QemuConsole *con, int x, int y) DisplayState *s = con->ds; struct DisplayChangeListener *dcl; -if (con != active_console) { +if (!qemu_console_is_visible(con)) { return; } QLIST_FOREACH(dcl, &s->listeners, next) { @@ -1415,7 +1411,7 @@ void dpy_text_update(QemuConsole *con, int x, int y, int w, int h) DisplayState *s = con->ds; struct DisplayChangeListener *dcl; -if (con != active_console) { +if (!qemu_console_is_visible(con)) { return; } QLIST_FOREACH(dcl, &s->listeners, next) { @@ -1430,7 +1426,7 @@ void dpy_text_resize(QemuConsole *con, int w, int h) DisplayState *s = con->ds; struct DisplayChangeListener *dcl; -if (con != active_console) { +if (!qemu_console_is_visible(con)) { return; } QLIST_FOREACH(dcl, &s->listeners, next) { @@
Re: [Qemu-devel] [PATCH 0/5] for spice post load char device hook
Please ignore this email, sent before I remembered to squash one of the patches. > This reworks my former patch > (http://patchwork.ozlabs.org/patch/227678/ - sorry > Hans, can't find the version you posted) per Gerd's suggestion. > Specifically it > adds a new qemu_chr_fe_post_load api that is called by the front end > and > implemented by the backend. virtio-console implements it, adding it's > own hook > which is called by virtio-serial-bus upon post_load from the timer, > so that > qemu_chr_fe_post_load is called when the vm is already in the running > state. > This makes the spice-qemu-char usage very simple by not requiring yet > another > timer. > > Alon Levy (5): > char: add a post_load callback > merge to char.h > virtio-serial: add a post_load callback implemented by port > virtio-console: implement post_load to call to > qemu_chr_fe_post_load > spice-qemu-char: register interface on post load > > hw/virtio-console.c| 11 +++ > hw/virtio-serial-bus.c | 5 + > hw/virtio-serial.h | 2 ++ > include/char/char.h| 9 + > qemu-char.c| 7 +++ > spice-qemu-char.c | 9 + > 6 files changed, 43 insertions(+) > > -- > 1.8.1.4 > > >
[Qemu-devel] [PATCH 2/4] virtio-serial: add a post_load callback implemented by port
Signed-off-by: Alon Levy --- hw/virtio-serial-bus.c | 5 + hw/virtio-serial.h | 2 ++ 2 files changed, 7 insertions(+) diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c index ab7168e..b2a50cb 100644 --- a/hw/virtio-serial-bus.c +++ b/hw/virtio-serial-bus.c @@ -584,12 +584,14 @@ static void virtio_serial_post_load_timer_cb(void *opaque) VirtIOSerial *s = opaque; VirtIOSerialPort *port; uint8_t host_connected; +VirtIOSerialPortClass *vsc; if (!s->post_load) { return; } for (i = 0 ; i < s->post_load->nr_active_ports; ++i) { port = s->post_load->connected[i].port; +vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port); host_connected = s->post_load->connected[i].host_connected; if (host_connected != port->host_connected) { /* @@ -599,6 +601,9 @@ static void virtio_serial_post_load_timer_cb(void *opaque) send_control_event(s, port->id, VIRTIO_CONSOLE_PORT_OPEN, port->host_connected); } +if (vsc->post_load) { +vsc->post_load(port); +} } g_free(s->post_load->connected); qemu_free_timer(s->post_load->timer); diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h index 484dcfe..24d70ed 100644 --- a/hw/virtio-serial.h +++ b/hw/virtio-serial.h @@ -107,6 +107,8 @@ typedef struct VirtIOSerialPortClass { */ ssize_t (*have_data)(VirtIOSerialPort *port, const uint8_t *buf, size_t len); + +void (*post_load)(VirtIOSerialPort *port); } VirtIOSerialPortClass; /* -- 1.8.1.4
[Qemu-devel] [PATCH 14/23] console: move gui_update+gui_setup_refresh from vl.c into console.c
Pure code motion, no functional changes. Signed-off-by: Gerd Hoffmann --- include/ui/console.h |2 -- ui/console.c | 50 ++ vl.c | 49 - 3 files changed, 50 insertions(+), 51 deletions(-) diff --git a/include/ui/console.h b/include/ui/console.h index d6e3e92..d92626b 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -213,8 +213,6 @@ static inline int is_buffer_shared(DisplaySurface *surface) return !(surface->flags & QEMU_ALLOCATED_FLAG); } -void gui_setup_refresh(DisplayState *ds); - void register_displaychangelistener(DisplayState *ds, DisplayChangeListener *dcl); void unregister_displaychangelistener(DisplayChangeListener *dcl); diff --git a/ui/console.c b/ui/console.c index 771f155..8e8f918 100644 --- a/ui/console.c +++ b/ui/console.c @@ -166,6 +166,56 @@ static void text_console_do_init(CharDriverState *chr, DisplayState *ds); static void dpy_gfx_switch_surface(DisplayState *ds, DisplaySurface *surface); +static void gui_update(void *opaque) +{ +uint64_t interval = GUI_REFRESH_INTERVAL; +DisplayState *ds = opaque; +DisplayChangeListener *dcl; + +dpy_refresh(ds); + +QLIST_FOREACH(dcl, &ds->listeners, next) { +if (dcl->gui_timer_interval && +dcl->gui_timer_interval < interval) { +interval = dcl->gui_timer_interval; +} +} +qemu_mod_timer(ds->gui_timer, interval + qemu_get_clock_ms(rt_clock)); +} + +static void gui_setup_refresh(DisplayState *ds) +{ +DisplayChangeListener *dcl; +bool need_timer = false; +bool have_gfx = false; +bool have_text = false; + +QLIST_FOREACH(dcl, &ds->listeners, next) { +if (dcl->ops->dpy_refresh != NULL) { +need_timer = true; +} +if (dcl->ops->dpy_gfx_update != NULL) { +have_gfx = true; +} +if (dcl->ops->dpy_text_update != NULL) { +have_text = true; +} +} + +if (need_timer && ds->gui_timer == NULL) { +ds->gui_timer = qemu_new_timer_ms(rt_clock, gui_update, ds); +qemu_mod_timer(ds->gui_timer, qemu_get_clock_ms(rt_clock)); +} +if (!need_timer && ds->gui_timer != NULL) { +qemu_del_timer(ds->gui_timer); +qemu_free_timer(ds->gui_timer); +ds->gui_timer = NULL; +} + +ds->have_gfx = have_gfx; +ds->have_text = have_text; +} + void graphic_hw_update(QemuConsole *con) { if (!con) { diff --git a/vl.c b/vl.c index c5eb9a7..12ddd83 100644 --- a/vl.c +++ b/vl.c @@ -1615,55 +1615,6 @@ MachineInfoList *qmp_query_machines(Error **errp) /***/ /* main execution loop */ -static void gui_update(void *opaque) -{ -uint64_t interval = GUI_REFRESH_INTERVAL; -DisplayState *ds = opaque; -DisplayChangeListener *dcl; - -dpy_refresh(ds); - -QLIST_FOREACH(dcl, &ds->listeners, next) { -if (dcl->gui_timer_interval && -dcl->gui_timer_interval < interval) -interval = dcl->gui_timer_interval; -} -qemu_mod_timer(ds->gui_timer, interval + qemu_get_clock_ms(rt_clock)); -} - -void gui_setup_refresh(DisplayState *ds) -{ -DisplayChangeListener *dcl; -bool need_timer = false; -bool have_gfx = false; -bool have_text = false; - -QLIST_FOREACH(dcl, &ds->listeners, next) { -if (dcl->ops->dpy_refresh != NULL) { -need_timer = true; -} -if (dcl->ops->dpy_gfx_update != NULL) { -have_gfx = true; -} -if (dcl->ops->dpy_text_update != NULL) { -have_text = true; -} -} - -if (need_timer && ds->gui_timer == NULL) { -ds->gui_timer = qemu_new_timer_ms(rt_clock, gui_update, ds); -qemu_mod_timer(ds->gui_timer, qemu_get_clock_ms(rt_clock)); -} -if (!need_timer && ds->gui_timer != NULL) { -qemu_del_timer(ds->gui_timer); -qemu_free_timer(ds->gui_timer); -ds->gui_timer = NULL; -} - -ds->have_gfx = have_gfx; -ds->have_text = have_text; -} - struct vm_change_state_entry { VMChangeStateHandler *cb; void *opaque; -- 1.7.9.7
[Qemu-devel] [PATCH 1/4] char: add a post_load callback
Signed-off-by: Alon Levy --- include/char/char.h | 12 qemu-char.c | 7 +++ 2 files changed, 19 insertions(+) diff --git a/include/char/char.h b/include/char/char.h index 0326b2a..0fdcaf9 100644 --- a/include/char/char.h +++ b/include/char/char.h @@ -70,6 +70,7 @@ struct CharDriverState { void (*chr_set_echo)(struct CharDriverState *chr, bool echo); void (*chr_guest_open)(struct CharDriverState *chr); void (*chr_guest_close)(struct CharDriverState *chr); +void (*chr_post_load)(struct CharDriverState *chr, int connected); void *opaque; int idle_tag; char *label; @@ -144,6 +145,17 @@ void qemu_chr_fe_open(struct CharDriverState *chr); void qemu_chr_fe_close(struct CharDriverState *chr); /** + * @qemu_chr_fe_post_load: + * + * Indicate to backend that a migration has just completed. Must be called when + * the vm is in the running state. + * + * @connected true if frontend is still connected after migration, false + * otherwise. + */ +void qemu_chr_fe_post_load(struct CharDriverState *chr, int connected); + +/** * @qemu_chr_fe_printf: * * Write to a character backend using a printf style interface. diff --git a/qemu-char.c b/qemu-char.c index 4e011df..42c911f 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -3390,6 +3390,13 @@ void qemu_chr_fe_open(struct CharDriverState *chr) } } +void qemu_chr_fe_post_load(struct CharDriverState *chr, int connected) +{ +if (chr->chr_post_load) { +chr->chr_post_load(chr, connected); +} +} + void qemu_chr_fe_close(struct CharDriverState *chr) { if (chr->chr_guest_close) { -- 1.8.1.4
[Qemu-devel] [PATCH 20/23] console: allow pinning displaychangelisteners to consoles
DisplayChangeListener gets a new QemuConsole field, which can be set to non-NULL before registering. This will pin the QemuConsole, so that particular DisplayChangeListener will not follow console switches. spice+gtk (which don't support text console input anyway) are switched over to be pinned to console 0, which usually is the graphical display. Signed-off-by: Gerd Hoffmann --- hw/qxl.c |2 +- include/ui/console.h |2 + include/ui/spice-display.h |1 - ui/console.c | 103 +++- ui/gtk.c |3 +- ui/spice-display.c | 11 ++--- 6 files changed, 84 insertions(+), 38 deletions(-) diff --git a/hw/qxl.c b/hw/qxl.c index 36f18fd..320c017 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -2061,7 +2061,6 @@ static int qxl_init_primary(PCIDevice *dev) portio_list_add(qxl_vga_port_list, pci_address_space_io(dev), 0x3b0); vga->con = graphic_console_init(&qxl_ops, qxl); -qxl->ssd.con = vga->con, qemu_spice_display_init_common(&qxl->ssd); rc = qxl_init_common(qxl); @@ -2070,6 +2069,7 @@ static int qxl_init_primary(PCIDevice *dev) } qxl->ssd.dcl.ops = &display_listener_ops; +qxl->ssd.dcl.con = vga->con; ds = qemu_console_displaystate(vga->con); register_displaychangelistener(ds, &qxl->ssd.dcl); return rc; diff --git a/include/ui/console.h b/include/ui/console.h index bcd0139..e591d74 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -178,6 +178,7 @@ struct DisplayChangeListener { uint64_t update_interval; const DisplayChangeListenerOps *ops; DisplayState *ds; +QemuConsole *con; QLIST_ENTRY(DisplayChangeListener) next; }; @@ -282,6 +283,7 @@ void graphic_hw_update(QemuConsole *con); void graphic_hw_invalidate(QemuConsole *con); void graphic_hw_text_update(QemuConsole *con, console_ch_t *chardata); +QemuConsole *qemu_console_lookup_by_index(unsigned int index); bool qemu_console_is_visible(QemuConsole *con); bool qemu_console_is_graphic(QemuConsole *con); bool qemu_console_is_fixedsize(QemuConsole *con); diff --git a/include/ui/spice-display.h b/include/ui/spice-display.h index 7a20fc4..a46bc80 100644 --- a/include/ui/spice-display.h +++ b/include/ui/spice-display.h @@ -71,7 +71,6 @@ typedef struct SimpleSpiceDisplay SimpleSpiceDisplay; typedef struct SimpleSpiceUpdate SimpleSpiceUpdate; struct SimpleSpiceDisplay { -QemuConsole *con; DisplaySurface *ds; DisplayChangeListener dcl; void *buf; diff --git a/ui/console.c b/ui/console.c index 0f2fea5..d78552e 100644 --- a/ui/console.c +++ b/ui/console.c @@ -117,6 +117,7 @@ struct QemuConsole { console_type_t console_type; DisplayState *ds; DisplaySurface *surface; +int dcls; /* Graphic console state. */ const GraphicHwOps *hw_ops; @@ -172,8 +173,6 @@ static QemuConsole *consoles[MAX_CONSOLES]; static int nb_consoles = 0; static void text_console_do_init(CharDriverState *chr, DisplayState *ds); -static void dpy_gfx_switch_surface(DisplayState *ds, - DisplaySurface *surface); static void dpy_refresh(DisplayState *s); static void gui_update(void *opaque) @@ -309,7 +308,7 @@ write_err: void qmp_screendump(const char *filename, Error **errp) { -QemuConsole *con = consoles[0]; +QemuConsole *con = qemu_console_lookup_by_index(0); DisplaySurface *surface; graphic_hw_update(con); @@ -1016,13 +1015,14 @@ static void console_putchar(QemuConsole *s, int ch) void console_select(unsigned int index) { +DisplayChangeListener *dcl; QemuConsole *s; if (index >= MAX_CONSOLES) return; trace_console_select(index); -s = consoles[index]; +s = qemu_console_lookup_by_index(index); if (s) { DisplayState *ds = s->ds; @@ -1031,7 +1031,14 @@ void console_select(unsigned int index) } active_console = s; if (ds->have_gfx) { -dpy_gfx_switch_surface(ds, s->surface); +QLIST_FOREACH(dcl, &ds->listeners, next) { +if (dcl->con != NULL) { +continue; +} +if (dcl->ops->dpy_gfx_switch) { +dcl->ops->dpy_gfx_switch(dcl, s->surface); +} +} dpy_gfx_update(s, 0, 0, surface_width(s->surface), surface_height(s->surface)); } @@ -1286,12 +1293,20 @@ void qemu_free_displaysurface(DisplaySurface *surface) void register_displaychangelistener(DisplayState *ds, DisplayChangeListener *dcl) { +QemuConsole *con; + trace_displaychangelistener_register(dcl, dcl->ops->dpy_name); dcl->ds = ds; QLIST_INSERT_HEAD(&ds->listeners, dcl, next); gui_setup_refresh(ds); -if (dcl->ops->dpy_gfx_switch && active_console) { -dcl->ops->dpy_gfx_switch(dcl, active_consol
[Qemu-devel] [PATCH 3/4] virtio-console: implement post_load to call to qemu_chr_fe_post_load
Signed-off-by: Alon Levy --- hw/virtio-console.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/hw/virtio-console.c b/hw/virtio-console.c index e2d1c58..a6ff2f7 100644 --- a/hw/virtio-console.c +++ b/hw/virtio-console.c @@ -88,6 +88,16 @@ static void guest_close(VirtIOSerialPort *port) qemu_chr_fe_close(vcon->chr); } +static void post_load(VirtIOSerialPort *port) +{ +VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port); + +if (!vcon->chr) { +return; +} +qemu_chr_fe_post_load(vcon->chr, port->guest_connected); +} + /* Readiness of the guest to accept data on a port */ static int chr_can_read(void *opaque) { @@ -177,6 +187,7 @@ static void virtserialport_class_init(ObjectClass *klass, void *data) k->have_data = flush_buf; k->guest_open = guest_open; k->guest_close = guest_close; +k->post_load = post_load; dc->props = virtserialport_properties; } -- 1.8.1.4
Re: [Qemu-devel] [PATCH V3 WIP 3/3] disable vhost_verify_ring_mappings check
On Tue, Mar 19, 2013 at 06:57:08PM -0700, Nicholas A. Bellinger wrote: > On Tue, 2013-03-19 at 09:40 +0100, Stefan Hajnoczi wrote: > > On Tue, Mar 19, 2013 at 08:34:45AM +0800, Asias He wrote: > > > --- > > > hw/vhost.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/hw/vhost.c b/hw/vhost.c > > > index 4d6aee3..0c52ec4 100644 > > > --- a/hw/vhost.c > > > +++ b/hw/vhost.c > > > @@ -421,10 +421,12 @@ static void vhost_set_memory(MemoryListener > > > *listener, > > > return; > > > } > > > > > > +#if 0 > > > if (dev->started) { > > > r = vhost_verify_ring_mappings(dev, start_addr, size); > > > assert(r >= 0); > > > } > > > +#endif > > > > Please add a comment to explain why. > > > > Btw, the output that Asias added in the failure case at the behest of > MST is here: > > http://www.spinics.net/lists/target-devel/msg04077.html Yes I suspected we could get l > ring_size, but this is not the case here. > MST seemed to think it may be a bug in cpu_physical_memory_map, but as > this worked with the original vhost-scsi code it would seem to indicate > something else at fault.. > > I'll be comparing what the original code did vs. vhost-scsi-pci to track > this down, but any extra ideas to track is down is appreciated. ;) > > --nab
[Qemu-devel] [PATCH 4/4] spice-qemu-char: register interface on post load
The target has not seen the guest_connected event via spice_chr_guest_open or spice_chr_write, and so spice server wrongly assumes there is no agent active, while the client continues to send motion events only by the agent channel, which the server ignores. The net effect is that the mouse is static in the guest. By registering the interface on post load spice server will pass on the agent messages fixing the mouse behavior after migration. RHBZ #725965 Signed-off-by: Alon Levy --- spice-qemu-char.c | 9 + 1 file changed, 9 insertions(+) diff --git a/spice-qemu-char.c b/spice-qemu-char.c index 8a9236d..c6aed58 100644 --- a/spice-qemu-char.c +++ b/spice-qemu-char.c @@ -205,6 +205,14 @@ static void print_allowed_subtypes(void) fprintf(stderr, "\n"); } +static void spice_chr_post_load(struct CharDriverState *chr, + int connected) +{ +if (connected) { +spice_chr_guest_open(chr); +} +} + static CharDriverState *chr_open(const char *subtype) { CharDriverState *chr; @@ -220,6 +228,7 @@ static CharDriverState *chr_open(const char *subtype) chr->chr_close = spice_chr_close; chr->chr_guest_open = spice_chr_guest_open; chr->chr_guest_close = spice_chr_guest_close; +chr->chr_post_load = spice_chr_post_load; QLIST_INSERT_HEAD(&spice_chars, s, next); -- 1.8.1.4
[Qemu-devel] QOM-ify QemuConsoles ...
Hi, So, the big qemu console data structure overhauls is done, patches just posted to the list. I think the next logical step ahead is to QOM-ify the QemuConsoles, so we can link the QemuConsole to the thing actually backing it. For a graphical console that would be the emlated graphic device. For a text console it would be the serial line or monitor hooked up to it. With this in place we should be able to answer questions like "which device backs this QemuConsole" by inspecting the object tree and handle requests like "do a screendump of this device please". It will also be useful to setup input routing: "pointer events from $this QemuConsole should to $that virtual input device". Hints how to do that best? Pointers to sample code to look at? From a brief look it seems we only QOM-ified emulated devices and not host-side objects yet ... thanks, Gerd
Re: [Qemu-devel] [PATCH] HLFS driver for QEMU
On Wed, Mar 20, 2013 at 04:43:17PM +0800, harryxiyou wrote: > On Mon, Mar 18, 2013 at 11:16 PM, Stefan Hajnoczi wrote: > [...] > > I looked at the Google Code project before, it looks like a repo that > > a few people are hacking on. The site is developer-focussed and there > > is no evidence of users. This is why I asked about the background of > > the community. > > > > Cloudxy is developing and we will popularize it. Most important for QEMU is easy building and testing, so let's focus on that. Ceph and GlusterFS have packages available in popular distros. This allows us to hook them into QEMU's buildbot (http://buildbot.b1-systems.de/qemu/grid) so their code does not bitrot. Sheepdog's QEMU block driver has no external dependencies so it's even easier to build. I'd like to see packages for popular distros and a "Getting Started" page for users instead of developers. Then I can follow the steps to set up HLFS using the packages and the QEMU buildbot can build the HLFS block driver. Stefan
[Qemu-devel] [PATCH] append the terminating '\0' to bootorder string
Problem was introduced in commit c8a6ae8b. The last terminating '\0' was lost, use the right length 5 ("HALT\0"). Reported-by: Gerd Hoffmann Signed-off-by: Amos Kong --- vl.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/vl.c b/vl.c index aeed7f4..d3172ea 100644 --- a/vl.c +++ b/vl.c @@ -1273,9 +1273,9 @@ char *get_boot_devices_list(size_t *size) if (boot_strict && *size > 0) { list[total-1] = '\n'; -list = g_realloc(list, total + 4); -memcpy(&list[total], "HALT", 4); -*size = total + 4; +list = g_realloc(list, total + 5); +memcpy(&list[total], "HALT", 5); +*size = total + 5; } return list; } -- 1.7.1
Re: [Qemu-devel] [Xen-devel] [PATCHv3] QEMU(upstream): Disable xen's use of O_DIRECT by default as it results in crashes.
On Tue, 19 Mar 2013, Alex Bligh wrote: > Stefano, George, > > > This patch only impacts the PV backend in QEMU, not the IDE interface. > > PV frontends and backends always disconnect and reconnect during > > save/restore. > > So we can be *certain* that bdrv_close at the sender side is called > > before the new connection is established at the receiver side. > > > > Unfortunately the QEMU PV backend opens the disk during initialization, > > so before the actual connection is established (blk_init). > > > > Therefore I think that the current change is not safe, but it is pretty > > easy to make it safe. > > You just need to move the call to blk_open from blk_init to blk_connect. > > Actually you can move most of blk_init to blk_connect, you just need to > > keep the 4 xenstore_write_be_int at the end of the function. > > Stefano: do you want me to come up with a patch to do this? Yes, please. > --On 19 March 2013 15:29:36 + George Dunlap > wrote: > > > Alex, the title of the changelog should certainly be more specific, so > > people aren't confused. Maybe something like the following? > > > > "Disable use of O_DIRECT when acting as a Xen PV backend" > > > > And emphasizing in the changelog that this has no effect on qemu when > > acting as a device model for Xen? > > Sure, I can do that. > > However, my understanding (possibly incorrect) is that when qemu is > acting as a device model for xen (i.e. emulated disks) it doesn't > use O_DIRECT anyway. I am aware this is a different problem! That is true. My guess is that nobody really migrates HVM guests without PV drivers installed (it's not even possible on XenServer but xl let you do that if you want to). When the PV drivers initialize at boot time, the IDE disk is closed. Therefore we wouldn't have this problem. Maybe we should prevent HVM guest migration without PV drivers with xl too. Ian, what do you think?
Re: [Qemu-devel] [PATCH 00/35] hw/ reorganization, part 2
On 20 March 2013 00:00, Paolo Bonzini wrote: > ARM is already quite good in that respect. However, until all > architectures are converted cpu_*_init needs to remain because of > user-mode targets (where the CPUs are created by common code, there is > no board to encapsulate the target-specific differences). IMHO waiting > for the demise of cpu_*_init is putting the cart before the horse, or > another similar proverb. As I say, I don't object to you moving the cpu init code into the a*mpcore containers. I do object to you moving the a*mpcore containers into hw/arm, and so transitively I object to changes to the containers made only with the intent to cause them to move into hw/arm. > (Also, I don't see much difference between using a function in > target-ARCH and using a TYPE_FOO define from target-ARCH. They are the > same thing masked through RTTI. hw/ARCH should really be the bridge > between target-ARCH and hw/everything-else, and a*mpcore.c fits in that > description). Basically I don't like the way your categorization scheme seems to be "kind of device, except for stuff in hw/ARCH which has a completely different rationale". I'm OK with hw/ARCH having board models, because that's really "kind of device", it's just split what ought to be hw/boards into a bunch of separate directories. And I've accepted having some of the random "needs fixing and splitting and chucking back out of hw/ARCH" code as a temporary measure. But please stop trying to push stuff into hw/ARCH rather than categorising it properly. I feel like I'm arguing round in circles here. -- PMM
Re: [Qemu-devel] [Xen-devel] [PATCHv3] QEMU(upstream): Disable xen's use of O_DIRECT by default as it results in crashes.
On 20/03/13 10:24, Stefano Stabellini wrote: That is true. My guess is that nobody really migrates HVM guests without PV drivers installed (it's not even possible on XenServer but xl let you do that if you want to). When the PV drivers initialize at boot time, the IDE disk is closed. Therefore we wouldn't have this problem. Maybe we should prevent HVM guest migration without PV drivers with xl too. Ian, what do you think? We should be able to make it safe if we just make sure that qemu does a metadata sync / FS sync when we ask qemu to write the save file. -George
Re: [Qemu-devel] [PATCH][RFC 0/14] implement power chip
On 20 March 2013 00:56, li guang wrote: > 在 2013-03-19二的 10:15 +,Peter Maydell写道: >> The point is that how exactly power controllers connect >> to devices, and which devices respond to reset/suspend/etc >> is a property of the individual machine being modelled. >> An x86 PC will be different from an ARM devboard which is >> different again from the Exynos4 ARM SoC. So to allow this >> flexibility, you have to let the machine model do the configuration, >> which you do by having the model wire up the power controller >> to the devices in the same way it's done on hardware. > > agree, originally, I made all devices can realize the power > state callbacks, e.g. if one can do suspend, then it will > realize DeviceState::suspend, so if system go to suspend, > this method will be called. > do you want some explicit way to configure for machine's > devices if they can support power state changes? The devices should just implement appropriate signals/connections if they have a means of talking to a power controller, and the board model should wire them up. That's all. >> >> Hardware does it with signals, so should we. >> > >> > can these signals be viewed as the calling of corresponding methods? >> >> In some ways, they are -- but the wiring up of the source of >> the call to the implementation is done at runtime as devices >> are connected together. > > So, can I go ahead to do this work? Well, it's not for me to say what you should do, but you still seem to be trying to do this with device methods, which (as I've argued above) I think is the wrong approach. -- PMM
Re: [Qemu-devel] [PATCH v3] Use proper term in TCG README
On 20 March 2013 03:42, 陳韋任 (Wei-Ren Chen) wrote: > In TCG, "target" means the host architecture for which TCG generates > the code. Using "guest" rather than "target" to make the document more > consistent. > > Signed-off-by: Chen Wei-Ren Reviewed-by: Peter Maydell -- PMM
Re: [Qemu-devel] [PATCH V2 0/3] ARM: Misc ARM big-endian bug fixes
On 20 March 2013 09:28, Fabien Chouteau wrote: > On 03/19/2013 12:30 PM, Peter Maydell wrote: >> On 19 March 2013 11:21, Fabien Chouteau wrote: >>> Fabien Chouteau (3): >>> QAPI: Add ARMEB target-type >>> Add default config for armeb-softmmu >> >> Hi Fabien. I'm afraid I don't want to take these two unless they >> come attached to an actual cpu and board model that use them. >> Otherwise they're really just half-a-feature, I think. >> > > Well these patches are very small, but I think it can help people to get > started. I submitted them because I thought it was a regression to not > be able to build qemu-system-armeb. No, we deliberately don't build qemu-system-armeb because we don't have any implemented machines which are big-endian ARM. When the first machine is added to QEMU the patch set adding that support should turn on building of qemu-system-armeb (which would then support just that one machine type). thanks -- PMM
Re: [Qemu-devel] [PATCH v14 1/4] add a new runstate: RUN_STATE_GUEST_PANICKED
Il 20/03/2013 09:58, Markus Armbruster ha scritto: > Let's examine the other transitions to RUN_STATE_PAUSED, and whether > they can now come from RUN_STATE_GUEST_PANICKED: > > * process_incoming_migration_co() > > No, because we're in RUN_STATE_INMIGRATE here, aren't we? Juan? Yes. > * qmp_stop() > > No, because vm_stop() calls do_vm_stop() to do the actual state > transition, which protects it by runstate_is_running(). > > We can ignore the conditional, it merely punts the vm_stop() to the > main loop. > > Next question: RUN_STATE_INTERNAL_ERROR and RUN_STATE_SHUTDOWN may go to > RUN_STATE_FINISH_MIGRATE, but RUN_STATE_GUEST_PANICKED may not. Why? RUN_STATE_FINISH_MIGRATE is reached with vm_stop_force_state, so every state can go there. Next question: why doesn't the switch to RUN_STATE_SAVE_VM use vm_stop_force_state? Next question: almost all states go to RUN_STATE_FINISH_MIGRATE, the same would hold for RUN_STATE_SAVE_VM if it started using vm_stop_force_state. There are few exceptions, and I'm not even sure all of them are correct (why can't RUN_STATE_DEBUG go to RUN_STATE_FINISH_MIGRATE?). Should vm_stop_force_state override the runstate check (either directly, or by interposing a transition to RUN_STATE_PAUSED? The few outliers can be handled with manually-placed assertions. Paolo
Re: [Qemu-devel] [PATCH] block: fix bdrv_exceed_iops_limits wait computation
Reviewed-by: Zhi Yong Wu On Wed, 2013-03-20 at 10:12 +0100, Benoît Canet wrote: > This patch fix an I/O throttling behavior triggered by limiting at 150 iops > and running a load of 50 threads doing random pwrites on a raw virtio device. > > After a few moments the iops count start to oscillate steadily between 0 and a > value upper than the limit. > > As the load keep running the period and the amplitude of the oscillation > increase until io bursts reaching the physical storage max iops count are > done. > > These bursts are followed by guest io starvation. > > As the period of this oscillation cycle is increasing the cause must be a > computation error leading to increase slowly the wait time. > > This patch make the wait time a bit smaller and tests confirm that it solves > the oscillating behavior. > > Signed-off-by: Benoit Canet > --- > block.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/block.c b/block.c > index 0a062c9..455d8b0 100644 > --- a/block.c > +++ b/block.c > @@ -3739,7 +3739,7 @@ static bool bdrv_exceed_iops_limits(BlockDriverState > *bs, bool is_write, > } > > /* Calc approx time to dispatch */ > -wait_time = (ios_base + 1) / iops_limit; > +wait_time = ios_base / iops_limit; > if (wait_time > elapsed_time) { > wait_time = wait_time - elapsed_time; > } else { -- Regards, Zhi Yong Wu
Re: [Qemu-devel] [PATCH 00/35] hw/ reorganization, part 2
Il 20/03/2013 11:30, Peter Maydell ha scritto: > Basically I don't like the way your categorization scheme seems > to be "kind of device, except for stuff in hw/ARCH which has > a completely different rationale". I'm OK with hw/ARCH having > board models, because that's really "kind of device", it's > just split what ought to be hw/boards into a bunch of separate > directories. And I've accepted having some of the random "needs > fixing and splitting and chucking back out of hw/ARCH" code as > a temporary measure. But please stop trying to push stuff into > hw/ARCH rather than categorising it properly. I feel like I'm > arguing round in circles here. Ok, I guess I'll add hw/cpu. Paolo
Re: [Qemu-devel] [PATCH v14 1/4] add a new runstate: RUN_STATE_GUEST_PANICKED
Markus Armbruster writes: > Hu Tao writes: > >> The guest will be in this state when it is panicked. >> >> Signed-off-by: Wen Congyang >> Signed-off-by: Hu Tao >> --- >> include/sysemu/sysemu.h | 1 + >> qapi-schema.json| 5 - >> qmp.c | 3 +-- >> vl.c| 13 +++-- >> 4 files changed, 17 insertions(+), 5 deletions(-) >> >> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h >> index b19ec95..0412a8a 100644 >> --- a/include/sysemu/sysemu.h >> +++ b/include/sysemu/sysemu.h >> @@ -22,6 +22,7 @@ int qemu_uuid_parse(const char *str, uint8_t *uuid); >> bool runstate_check(RunState state); >> void runstate_set(RunState new_state); >> int runstate_is_running(void); >> +bool runstate_needs_reset(void); >> typedef struct vm_change_state_entry VMChangeStateEntry; >> typedef void VMChangeStateHandler(void *opaque, int running, RunState >> state); >> >> diff --git a/qapi-schema.json b/qapi-schema.json >> index 28b070f..003cbf2 100644 >> --- a/qapi-schema.json >> +++ b/qapi-schema.json >> @@ -174,11 +174,14 @@ >> # @suspended: guest is suspended (ACPI S3) >> # >> # @watchdog: the watchdog action is configured to pause and has been >> triggered >> +# >> +# @guest-panicked: guest has been panicked as a result of guest OS panic >> ## >> { 'enum': 'RunState', >>'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused', >> 'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm', >> -'running', 'save-vm', 'shutdown', 'suspended', 'watchdog' ] } >> +'running', 'save-vm', 'shutdown', 'suspended', 'watchdog', >> +'guest-panicked' ] } >> >> ## >> # @SnapshotInfo > > RUN_STATE_GUEST_PANICKED is similar to RUN_STATE_INTERNAL_ERROR and > RUN_STATE_SHUTDOWN: the only way for the guest to continue is reset. > Correct? > >> diff --git a/qmp.c b/qmp.c >> index 55b056b..a1ebb5d 100644 >> --- a/qmp.c >> +++ b/qmp.c >> @@ -149,8 +149,7 @@ void qmp_cont(Error **errp) >> { >> Error *local_err = NULL; >> >> -if (runstate_check(RUN_STATE_INTERNAL_ERROR) || >> - runstate_check(RUN_STATE_SHUTDOWN)) { >> +if (runstate_needs_reset()) { >> error_set(errp, QERR_RESET_REQUIRED); >> return; >> } else if (runstate_check(RUN_STATE_SUSPENDED)) { >> diff --git a/vl.c b/vl.c >> index c03edf1..926822b 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -566,6 +566,7 @@ static const RunStateTransition >> runstate_transitions_def[] = { >> { RUN_STATE_RUNNING, RUN_STATE_SAVE_VM }, >> { RUN_STATE_RUNNING, RUN_STATE_SHUTDOWN }, >> { RUN_STATE_RUNNING, RUN_STATE_WATCHDOG }, >> +{ RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED }, >> >> { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING }, >> > > This permits runstate_set(RUN_STATE_GUEST_PANICKED) in > RUN_STATE_RUNNING. Used in PATCH 3/4. Good. > >> @@ -580,6 +581,8 @@ static const RunStateTransition >> runstate_transitions_def[] = { >> { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING }, >> { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE }, >> >> +{ RUN_STATE_GUEST_PANICKED, RUN_STATE_PAUSED }, >> + >> { RUN_STATE_MAX, RUN_STATE_MAX }, >> }; > > This permits runstate_set(RUN_STATE_PAUSED) in RUN_STATE_GUEST_PANICKED. > An example for such a transition is in the last patch hunk: main loop > resetting the guest. > > Let's examine the other transitions to RUN_STATE_PAUSED, and whether > they can now come from RUN_STATE_GUEST_PANICKED: > > * gdb_read_byte() > > No, because vm_stop() is protected by runstate_is_running() here. > > * gdb_chr_event() case CHR_EVENT_OPENED > > Can this happen in RUN_STATE_GUEST_PANICKED? Jan? > > What about RUN_STATE_INTERNAL_ERROR and RUN_STATE_SHUTDOWN? Nevermind this one. Like for qmp_stop() below, the actual state transition is protected by runstate_is_running(). > * gdb_sigterm_handler() > > No, because vm_stop() is protected by runstate_is_running() here. > > Aside: despite its name, this function handles SIGINT. Ugh. > > * process_incoming_migration_co() > > No, because we're in RUN_STATE_INMIGRATE here, aren't we? Juan? > > * qmp_stop() > > No, because vm_stop() calls do_vm_stop() to do the actual state > transition, which protects it by runstate_is_running(). > > We can ignore the conditional, it merely punts the vm_stop() to the > main loop. [...]
Re: [Qemu-devel] [Xen-devel] [PATCHv3] QEMU(upstream): Disable xen's use of O_DIRECT by default as it results in crashes.
Il 20/03/2013 11:37, George Dunlap ha scritto: >> That is true. My guess is that nobody really migrates HVM guests without >> PV drivers installed (it's not even possible on XenServer but xl let you >> do that if you want to). When the PV drivers initialize at boot time, >> the IDE disk is closed. Therefore we wouldn't have this problem. >> >> Maybe we should prevent HVM guest migration without PV drivers with xl >> too. Ian, what do you think? > > We should be able to make it safe if we just make sure that qemu does a > metadata sync / FS sync when we ask qemu to write the save file. To make it safe, just use cache=none. There are downsides (for example tmpfs does not support it), but perhaps you can use a global option or environment variable to toggle the behavior. Is xl still using the driver:subdriver:/path,readonly syntax? That's really inflexible compared to libvirt's XML... Paolo
Re: [Qemu-devel] [PATCH] HLFS driver for QEMU
On Wed, Mar 20, 2013 at 6:04 PM, Stefan Hajnoczi wrote: > On Wed, Mar 20, 2013 at 04:43:17PM +0800, harryxiyou wrote: >> On Mon, Mar 18, 2013 at 11:16 PM, Stefan Hajnoczi wrote: >> [...] >> > I looked at the Google Code project before, it looks like a repo that >> > a few people are hacking on. The site is developer-focussed and there >> > is no evidence of users. This is why I asked about the background of >> > the community. >> > >> >> Cloudxy is developing and we will popularize it. > > Most important for QEMU is easy building and testing, so let's focus on > that. Yup, this is wonderful ;-) > > Ceph and GlusterFS have packages available in popular distros. This > allows us to hook them into QEMU's buildbot > (http://buildbot.b1-systems.de/qemu/grid) so their code does not bitrot. We will make debian and RPM packages. > > Sheepdog's QEMU block driver has no external dependencies so it's even > easier to build. Yup. > > I'd like to see packages for popular distros and a "Getting Started" > page for users instead of developers. Then I can follow the steps to > set up HLFS using the packages and the QEMU buildbot can build the HLFS > block driver. > Actually, we have "Get started" wiki pages in Chinese and i will give them in English. You can visit Chinese "Get started" wiki pages by following link. http://code.google.com/p/cloudxy/wiki/HlfsUserManual We also have "one key install script", which is located here. http://cloudxy.googlecode.com/svn/branches/hlfs/person/zhangdianbo/scripts/ All the wiki pages for Cloudxy are here(In Chinese). http://code.google.com/p/cloudxy/w/list -- Thanks Harry Wei
Re: [Qemu-devel] [Xen-devel] [PATCHv3] QEMU(upstream): Disable xen's use of O_DIRECT by default as it results in crashes.
--On 20 March 2013 12:08:19 +0100 Paolo Bonzini wrote: To make it safe, just use cache=none. There are downsides (for example tmpfs does not support it), but perhaps you can use a global option or environment variable to toggle the behavior. If you don't use cache=none, you it is unsafe because of the page cache issue. If you use cache=none, it uses O_DIRECT, and it is unsafe because of the kernel issue. My preference would be to leave this can of worms to the end user. I'd rather take the risk on the first (as I'd rather domU broke than dom0) but that's because I always use network drives. Some documentation would be useful. -- Alex Bligh
Re: [Qemu-devel] [PATCH] QMP: TPM QMP and man page documentation updates
Corey Bryant writes: > On 03/19/2013 03:26 AM, Markus Armbruster wrote: >> [Note cc: Anthony for QAPI schema expertise] >> >> Stefan Berger writes: >> >>> On 03/18/2013 12:16 PM, Markus Armbruster wrote: Corey Bryant writes: > Signed-off-by: Corey Bryant > --- >qemu-options.hx | 3 ++- >qmp-commands.hx | 59 > + >2 files changed, 61 insertions(+), 1 deletion(-) > > diff --git a/qemu-options.hx b/qemu-options.hx > index 30fb85d..3b3cd0f 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -2237,7 +2237,8 @@ Backend type must be: >@option{passthrough}. > The specific backend type will determine the applicable > options. > -The @code{-tpmdev} option requires a @code{-device} option. > +The @code{-tpmdev} option creates the TPM backend and requires a > +@code{-device} option that specifies the TPM frontend interface model. > Options to each backend are described below. >diff --git a/qmp-commands.hx b/qmp-commands.hx > index b370060..4eda5ea 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -2721,18 +2721,77 @@ EQMP >.mhandler.cmd_new = qmp_marshal_input_query_tpm, >}, >+SQMP > +query-tpm > +- > + > +Return information about the TPM device. > + > +Arguments: None > + > +Example: > + > +-> { "execute": "query-tpm" } > +<- { "return": > + [ > + { "model": "tpm-tis", > + "tpm-options": > + { "type": "tpm-passthrough-options", > + "data": > + { "cancel-path": "/sys/class/misc/tpm0/device/cancel", > + "path": "/dev/tpm0" > + } > + }, > + "type": "passthrough", > + "id": "tpm0" > + } > + ] > + } > + > +EQMP > + "tpm-options" is a discriminated union. How is its discriminator "type" (here: "tpm-passthrough-options") related to the outer "type" (here: "passthrough")? >>> >>> It gives you similar information twice. So there is a direct >>> relationship between the two types. >> >> Awkward and undocumented. >> >> Relevant parts of qapi-schema.json: >> >> { 'enum': 'TpmType', 'data': [ 'passthrough' ] } >> >> { 'union': 'TpmTypeOptions', >> 'data': { 'tpm-passthrough-options' : 'TPMPassthroughOptions' } } >> >> { 'type': 'TPMInfo', >>'data': {'id': 'str', >> 'model': 'TpmModel', >> 'type': 'TpmType', >> 'tpm-options': 'TpmTypeOptions' } } >> >> Type Netdev solves the same problem more elegantly: >> >> { 'union': 'NetClientOptions', >>'data': { >> 'none': 'NetdevNoneOptions', >> 'nic': 'NetLegacyNicOptions', >> 'user': 'NetdevUserOptions', >> 'tap': 'NetdevTapOptions', >> 'socket': 'NetdevSocketOptions', >> 'vde': 'NetdevVdeOptions', >> 'dump': 'NetdevDumpOptions', >> 'bridge': 'NetdevBridgeOptions', >> 'hubport': 'NetdevHubPortOptions' } } >> >> { 'type': 'Netdev', >>'data': { >> 'id': 'str', >> 'opts': 'NetClientOptions' } } >> >> Uses the union's discriminator. Straightforward. >> >> Following Netdev precedence, we get: >> >> { 'union': 'TpmTypeOptions', >>'data': { 'passthrough' : 'TPMPassthroughOptions' } } >> >> { 'type': 'TPMInfo', >>'data': {'id': 'str', >> 'model': 'TpmModel', >> 'opts': 'TpmTypeOptions' } } >> > > I can send a patch for this update if you'd like. Yes, please! >> Duplication of type is gone. No need for documentation. >> >> Since enum TpmType is used elsewhere, it still gets duplicated in the >> union's discriminator. Anthony, is there a way to name the implicit >> discriminator enum type for reference elsewhere in the schema? >> > > Are you saying it still gets duplicated with this fix? I'm not sure > what you mean. A union in the schema implicitely defines an C enumeration type to be used for its discriminator. For instance, union TpmTypeOptions implicitely defines this one: typedef enum TpmTypeOptionsKind { TPM_TYPE_OPTIONS_KIND_TPM_PASSTHROUGH_OPTIONS = 0, TPM_TYPE_OPTIONS_KIND_MAX = 1, } TpmTypeOptionsKind; QAPI's discriminated union becomes a C struct containing the discriminator and the union of the members: struct TpmTypeOptions { TpmTypeOptionsKind kind; union { void *data; TPMPassthroughOptions * tpm_passthrough_options; }; }; Enum TpmType and type TpmInfo become: typedef enum TpmType { TPM_TYPE_PASSTHROUGH = 0, TPM_TYPE_MAX = 1, } TpmType; struct TPMInfo {
Re: [Qemu-devel] [PATCH] virtio-blk-x: fix configuration synchronization.
On Wed, Mar 20, 2013 at 10:00:01AM +0100, fred.kon...@greensocs.com wrote: > From: KONRAD Frederic > > The virtio-blk-x configuration is not in sync with virtio-blk configuration. > So this patch remove the virtio-blk-x configuration field, and use virtio-blk > one for setting the properties. > > This also remove a useless configuration copy in virtio_blk_device_init. > > Note that this is on top of "virtio-ccw queue as of 2013/03/20". Look fine, although I haven't looked into the latest virtio-ccw or virtio-blk refactoring. Stefan
Re: [Qemu-devel] [PATCH] qemu-sockets: Fix assertion failure
On Wed, 20 Mar 2013 09:39:34 +0100 Kevin Wolf wrote: > Am 19.03.2013 um 21:34 hat Luiz Capitulino geschrieben: > > On Wed, 06 Mar 2013 15:46:45 +0100 > > Laszlo Ersek wrote: > > > > > On 03/06/13 12:11, Kevin Wolf wrote: > > > > Am 06.03.2013 um 12:04 hat Paolo Bonzini geschrieben: > > > >> Il 06/03/2013 11:48, Kevin Wolf ha scritto: > > > >>> inet_connect_opts() tries all possible addrinfos returned by > > > >>> getaddrinfo(). If one fails with an error, the next one is tried. In > > > >>> this case, the Error should be discarded because the whole operation > > > >>> is > > > >>> successful if another addrinfo from the list succeeds; and if it > > > >>> doesn't, setting an already set Error will trigger an assertion > > > >>> failure. > > > >>> > > > >>> Signed-off-by: Kevin Wolf > > > >>> --- > > > >>> util/qemu-sockets.c | 8 > > > >>> 1 file changed, 8 insertions(+) > > > >>> > > > >>> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c > > > >>> index 1350ccc..32e609a 100644 > > > >>> --- a/util/qemu-sockets.c > > > >>> +++ b/util/qemu-sockets.c > > > >>> @@ -373,6 +373,14 @@ int inet_connect_opts(QemuOpts *opts, Error > > > >>> **errp, > > > >>> } > > > >>> > > > >>> for (e = res; e != NULL; e = e->ai_next) { > > > >>> + > > > >>> +/* Overwriting errors isn't allowed, so clear any error that > > > >>> may have > > > >>> + * occured in the previous iteration */ > > > >>> +if (error_is_set(errp)) { > > > >>> +error_free(*errp); > > > >>> +*errp = NULL; > > > >>> +} > > > >>> + > > > >>> if (connect_state != NULL) { > > > >>> connect_state->current_addr = e; > > > >>> } > > > >>> > > > >> > > > >> Should we also do nothing if errp is not NULL on entry? > > > > > > > > We could assert(!error_is_set(errp)) if we wanted. As soon as you've got > > > > an Error, you must return instead of calling more functions with the > > > > same error pointer. > > > > > > I think Luiz would suggest (*) to receive any error into a > > > NULL-initialized local_err pointer; do the logic above on local_err, and > > > just before returning, error_propagate() it to errp. > > > > Yes, I'd suggest that but it turns out that inet_connect_addr() error > > reporting was and still is confusing, which causes callers to use it > > incorrectly. > > > > This patch (which has been applied by Anthony) > > No, Anthony applied a different, but similar patch of his own. This is > why I don't feel particularly responsible for the specific problem any > more. > > How to do error handling with Error right is the only reason for me to > continue the discussion. I think we need a kind of "Error best practices" doc. > > solves the problem at > > hand but it also introduces a new issue: errors from inet_connect_addr() > > are only reported if they happen in the last loop interaction. Note that > > a few other errors other than 'couldn't connect' can happen. > > > Laszlo's comment seemed to have triggered a discussion around Error **, > > but this really has very little to do with it: the real problem is that > > inet_connect_addr() is too confusing. > > Maybe we need to discuss first what the intended behaviour even is. My > interpretation was this: We may have several addresses to try. If one of > them works, the function as a whole has succeeded and must not return an > error, neither in errp nor as -errno. If none of them succeeds, the > function has to return an error, and returning the error of the last > attempt is as good as the error of any other attempt. I agree. When I looked at the code yesterday I had the impression that several other errors where possible, which made me wonder if we shouldn't stop short the loop on non-"can't connect" type of errors. But looking at it again we have only socket() and connect() calls, and I'd expect that most (all?) non can't connect errors will happen in all loop iterations, which will cause the error to be reported. > > inet_connect_addr() has two users: inet_connect_opts() and > > wait_for_connect(), > > with this patch both of them are now ignoring errors from > > inet_connect_addr(). > > > > Suggested solution: refactor inet_connect_addr() to return an errno value. > > Callers use error_set() when they want to report an error upward. > > Doesn't change the problem that you need to know when to set a return > value != 0. So it doesn't help, but you'd lose some error information. My real point is that it's easier to check against errno to find out the error cause (compared to using Error for that).
Re: [Qemu-devel] [PATCH 21/23] gtk: custom cursor support
Gerd Hoffmann writes: > Makes gtk ui play nicely with qxl (and vmware_svga) > as you can actually see your pointer now ;) > > Signed-off-by: Gerd Hoffmann > --- > ui/gtk.c | 25 + > 1 file changed, 25 insertions(+) > > diff --git a/ui/gtk.c b/ui/gtk.c > index 7599ff4..512e974 100644 > --- a/ui/gtk.c > +++ b/ui/gtk.c > @@ -303,6 +303,29 @@ static void gd_refresh(DisplayChangeListener *dcl) > graphic_hw_update(dcl->con); > } > > +static void gd_mouse_set(DisplayChangeListener *dcl, > + int x, int y, int visible) > +{ > +/* should warp pointer to x, y here */ This is just a matter of doing: gdk_window_get_root_coords(window, x, y, &x_root, &y_root); gdk_display_warp_pointer(display, screen, x_root, y_root); > +} > + > +static void gd_cursor_define(DisplayChangeListener *dcl, > + QEMUCursor *c) > +{ > +GtkDisplayState *s = container_of(dcl, GtkDisplayState, dcl); > +GdkPixbuf *pixbuf; > +GdkCursor *cursor; > + > +pixbuf = gdk_pixbuf_new_from_data((guchar *)(c->data), > + GDK_COLORSPACE_RGB, true, 8, > + c->width, c->height, c->width * 4, > + NULL, NULL); > +cursor = gdk_cursor_new_from_pixbuf(gdk_display_get_default(), You should get the display from the drawing_area widget. > +pixbuf, c->hot_x, c->hot_y); > +gdk_window_set_cursor(s->drawing_area->window, cursor); > +g_object_unref(pixbuf); You should also dereference the cursor here. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 22/23] gtk: show a window for each graphical QemuConsole
Gerd Hoffmann writes: > Multihead support: For each graphical console we'll create a gtk > window, so with multiple graphics cards installed you get a gtk window > for each. vte tabs are attached to the console #0 window. This is neat but I'm not sure if the user experience is right. Each window would have independent grab. Each window would have independent full screen. I'm not sure there's an obviously right solution but it's worth discussing. I do like the idea of having two windows but perhaps we should make the second window have a no menu bar with a title that indicates that it's a secondary display? Regards, Anthony Liguori > --- > ui/gtk.c | 40 +++- > 1 file changed, 27 insertions(+), 13 deletions(-) > > diff --git a/ui/gtk.c b/ui/gtk.c > index 512e974..1c86054 100644 > --- a/ui/gtk.c > +++ b/ui/gtk.c > @@ -159,8 +159,6 @@ typedef struct GtkDisplayState > bool external_pause_update; > } GtkDisplayState; > > -static GtkDisplayState *global_state; > - > /** Utility Functions **/ > > static bool gd_is_grab_active(GtkDisplayState *s) > @@ -1210,7 +1208,7 @@ static void gd_connect_signals(GtkDisplayState *s) > G_CALLBACK(gd_leave_event), s); > } > > -static void gd_create_menus(GtkDisplayState *s) > +static void gd_create_menus(GtkDisplayState *s, bool vtetabs) > { > GtkStockItem item; > GtkAccelGroup *accel_group; > @@ -1302,11 +1300,13 @@ static void gd_create_menus(GtkDisplayState *s) > gtk_accel_map_add_entry("/View/VGA", GDK_KEY_1, GDK_CONTROL_MASK | > GDK_MOD1_MASK); > gtk_menu_shell_append(GTK_MENU_SHELL(s->view_menu), s->vga_item); > > -for (i = 0; i < nb_vcs; i++) { > -VirtualConsole *vc = &s->vc[i]; > +if (vtetabs) { > +for (i = 0; i < nb_vcs; i++) { > +VirtualConsole *vc = &s->vc[i]; > > -group = gd_vc_init(s, vc, i, group); > -s->nb_vcs++; > +group = gd_vc_init(s, vc, i, group); > +s->nb_vcs++; > +} > } > > separator = gtk_separator_menu_item_new(); > @@ -1336,14 +1336,13 @@ static const DisplayChangeListenerOps dcl_ops = { > .dpy_cursor_define = gd_cursor_define, > }; > > -void gtk_display_init(DisplayState *ds) > +static void gtk_display_init_one(DisplayState *ds, QemuConsole *con, > + bool vtetabs) > { > GtkDisplayState *s = g_malloc0(sizeof(*s)); > > -gtk_init(NULL, NULL); > - > s->dcl.ops = &dcl_ops; > -s->dcl.con = qemu_console_lookup_by_index(0); > +s->dcl.con = con; > > s->window = gtk_window_new(GTK_WINDOW_TOPLEVEL); > #if GTK_CHECK_VERSION(3, 2, 0) > @@ -1371,7 +1370,7 @@ void gtk_display_init(DisplayState *ds) > > gtk_notebook_append_page(GTK_NOTEBOOK(s->notebook), s->drawing_area, > gtk_label_new("VGA")); > > -gd_create_menus(s); > +gd_create_menus(s, vtetabs); > > gd_connect_signals(s); > > @@ -1400,6 +1399,21 @@ void gtk_display_init(DisplayState *ds) > gtk_widget_show_all(s->window); > > register_displaychangelistener(ds, &s->dcl); > +} > > -global_state = s; > +void gtk_display_init(DisplayState *ds) > +{ > +QemuConsole *con; > +int i = 0; > + > +gtk_init(NULL, NULL); > + > +con = qemu_console_lookup_by_index(i++); > +gtk_display_init_one(ds, con, true); > +while ((con = qemu_console_lookup_by_index(i++)) != NULL) { > +if (!qemu_console_is_graphic(con)) { > +break; > +} > +gtk_display_init_one(ds, con, false); > +} > } > -- > 1.7.9.7
Re: [Qemu-devel] [RFC PATCH RDMA support v4: 03/10] more verbose documentation of the RDMA transport
On Tue, Mar 19, 2013 at 06:52:59PM +0100, Paolo Bonzini wrote: > Il 19/03/2013 18:40, Michael R. Hines ha scritto: > > registration scheme would not work with cgroups because we would be > > attempting to pin zero pages (for no reason) that cgroups has already > > kicked out, which would defeat the purpose of using cgroups. > > Yeah, pinning would be a problem. > > > So, if I submit a separate patch to fix this, would you guys review it? > > (Using /dev/pagemap). > > Sorry about the ignorance, but what is /dev/pagemap? :) > > > Unless there is a better idea? Does KVM expose the necessary mappings? > > We could have the balloon driver track the pages. I and Michael had > some initial work a few months ago on extending the virtio-balloon spec > to allow this. It went nowhere, though. > > Still, at this point this is again an RDMA-specific problem, I don't > think it would be that bad if the first iterations of RDMA didn't > support ballooning/overcommit. > > Paolo My problem is with the protocol. If it assumes at the protocol level that everything is pinned down on the destination, we'll have to rework it all to make it really useful. -- MST
[Qemu-devel] [PATCH 1/2] libcacard: remove sql: prefix
For some reason, with sql:/ prefix, the PKCS11 modules are not loaded. This patch goes on top of Alon smartcard series. --- libcacard/vcard_emul_nss.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c index 6bad0b9..a0dd32d 100644 --- a/libcacard/vcard_emul_nss.c +++ b/libcacard/vcard_emul_nss.c @@ -898,7 +898,7 @@ vcard_emul_init(const VCardEmulOptions *options) if (options->nss_db) { rv = NSS_Init(options->nss_db); } else { -gchar *path, *db; +gchar *path; #ifndef _WIN32 path = g_strdup("/etc/pki/nssdb"); #else @@ -910,10 +910,8 @@ vcard_emul_init(const VCardEmulOptions *options) path = g_build_filename( g_get_system_config_dirs()[0], "pki", "nssdb", NULL); #endif -db = g_strdup_printf("sql:%s", path); -rv = NSS_Init(db); -g_free(db); +rv = NSS_Init(path); g_free(path); } if (rv != SECSuccess) { -- 1.8.1.1.439.g50a6b54
[Qemu-devel] [PATCH 2/2] libcacard: remove default libcoolkey loading
Use only the modules defined in the NSS database. --- libcacard/vcard_emul_nss.c | 17 + 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c index a0dd32d..7ed94af 100644 --- a/libcacard/vcard_emul_nss.c +++ b/libcacard/vcard_emul_nss.c @@ -875,7 +875,7 @@ VCardEmulError vcard_emul_init(const VCardEmulOptions *options) { SECStatus rv; -PRBool ret, has_readers = PR_FALSE, need_coolkey_module; +PRBool ret, has_readers = PR_FALSE; VReader *vreader; VReaderEmul *vreader_emul; SECMODListLock *module_lock; @@ -988,30 +988,15 @@ vcard_emul_init(const VCardEmulOptions *options) /* make sure we have some PKCS #11 module loaded */ module_lock = SECMOD_GetDefaultModuleListLock(); module_list = SECMOD_GetDefaultModuleList(); -need_coolkey_module = !has_readers; SECMOD_GetReadLock(module_lock); for (mlp = module_list; mlp; mlp = mlp->next) { SECMODModule *module = mlp->module; if (module_has_removable_hw_slots(module)) { -need_coolkey_module = PR_FALSE; break; } } SECMOD_ReleaseReadLock(module_lock); -if (need_coolkey_module) { -SECMODModule *module; -module = SECMOD_LoadUserModule( -(char *)"library=libcoolkeypk11.so name=Coolkey", -NULL, PR_FALSE); -if (module == NULL) { -return VCARD_EMUL_FAIL; -} -SECMOD_DestroyModule(module); /* free our reference, Module will still - * be on the list. - * until we destroy it */ -} - /* now examine all the slots, finding which should be readers */ /* We should control this with options. For now we mirror out any * removable hardware slot */ -- 1.8.1.1.439.g50a6b54
Re: [Qemu-devel] [PATCH 1/4] char: add a post_load callback
Alon Levy writes: > Signed-off-by: Alon Levy > --- > include/char/char.h | 12 > qemu-char.c | 7 +++ > 2 files changed, 19 insertions(+) > > diff --git a/include/char/char.h b/include/char/char.h > index 0326b2a..0fdcaf9 100644 > --- a/include/char/char.h > +++ b/include/char/char.h > @@ -70,6 +70,7 @@ struct CharDriverState { > void (*chr_set_echo)(struct CharDriverState *chr, bool echo); > void (*chr_guest_open)(struct CharDriverState *chr); > void (*chr_guest_close)(struct CharDriverState *chr); > +void (*chr_post_load)(struct CharDriverState *chr, int > connected); The character device layer should *not* be messing around with notifying migration state. I thought we previously discussed this? Just implement a migration hook in the spice code. Regards, Anthony Liguori > void *opaque; > int idle_tag; > char *label; > @@ -144,6 +145,17 @@ void qemu_chr_fe_open(struct CharDriverState *chr); > void qemu_chr_fe_close(struct CharDriverState *chr); > > /** > + * @qemu_chr_fe_post_load: > + * > + * Indicate to backend that a migration has just completed. Must be called > when > + * the vm is in the running state. > + * > + * @connected true if frontend is still connected after migration, false > + * otherwise. > + */ > +void qemu_chr_fe_post_load(struct CharDriverState *chr, int connected); > + > +/** > * @qemu_chr_fe_printf: > * > * Write to a character backend using a printf style interface. > diff --git a/qemu-char.c b/qemu-char.c > index 4e011df..42c911f 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -3390,6 +3390,13 @@ void qemu_chr_fe_open(struct CharDriverState *chr) > } > } > > +void qemu_chr_fe_post_load(struct CharDriverState *chr, int connected) > +{ > +if (chr->chr_post_load) { > +chr->chr_post_load(chr, connected); > +} > +} > + > void qemu_chr_fe_close(struct CharDriverState *chr) > { > if (chr->chr_guest_close) { > -- > 1.8.1.4
Re: [Qemu-devel] [PATCH v2 00/15] propagate Errors to do_device_add()
No longer applies cleanly, but git-am -3 takes care of everything except for a trivial #include conflict. I didn't test the result, though :)
Re: [Qemu-devel] [PATCH v2 02/15] do_device_add(): look up "device" opts list with qemu_find_opts_err()
Laszlo Ersek writes: > Conversion status (call chains covered or substituted by error propagation > marked with square brackets): > > do_device_add -> [qemu_find_opts -> error_report] > do_device_add -> qdev_device_add -> qerror_report > do_device_add -> qdev_device_add -> qbus_find -> qbus_find_recursive > -> qerror_report > do_device_add -> qdev_device_add -> qbus_find -> qerror_report > do_device_add -> qdev_device_add -> set_property -> qdev_prop_parse > -> qerror_report_err > > Signed-off-by: Laszlo Ersek > --- > hw/qdev-monitor.c |7 ++- > 1 files changed, 6 insertions(+), 1 deletions(-) > > diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c > index 56d66c3..bbdc90f 100644 > --- a/hw/qdev-monitor.c > +++ b/hw/qdev-monitor.c > @@ -590,15 +590,20 @@ void do_info_qdm(Monitor *mon, const QDict *qdict) > int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data) > { > Error *local_err = NULL; > +QemuOptsList *list; > QemuOpts *opts; > DeviceState *dev; > > -opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict, &local_err); > +list = qemu_find_opts_err("device", &local_err); > +if (!error_is_set(&local_err)) { > +opts = qemu_opts_from_qdict(list, qdict, &local_err); > +} > if (error_is_set(&local_err)) { > qerror_report_err(local_err); > error_free(local_err); > return -1; > } > + > if (!monitor_cur_is_qmp() && qdev_device_help(opts)) { > qemu_opts_del(opts); > return 0; } dev = qdev_device_add(opts, &local_err); /work/armbru/qemu/qdev-monitor.c:667:9: warning: ‘opts’ may be used uninitialized in this function [-Wmaybe-uninitialized] Not really, of course. We get here only if local_err is clear, and then we surely set opts. Recommend "fix" this warning by dropping this patch. qemu_find_opts("device") can't fail. We already assume that elsewhere, e.g. in drive_init(), balloon_parse(), virtcon_parse(), ...
Re: [Qemu-devel] [PATCH] block: fix bdrv_exceed_iops_limits wait computation
On Wed, Mar 20, 2013 at 10:12:14AM +0100, Benoît Canet wrote: > This patch fix an I/O throttling behavior triggered by limiting at 150 iops > and running a load of 50 threads doing random pwrites on a raw virtio device. > > After a few moments the iops count start to oscillate steadily between 0 and a > value upper than the limit. > > As the load keep running the period and the amplitude of the oscillation > increase until io bursts reaching the physical storage max iops count are > done. > > These bursts are followed by guest io starvation. > > As the period of this oscillation cycle is increasing the cause must be a > computation error leading to increase slowly the wait time. > > This patch make the wait time a bit smaller and tests confirm that it solves > the oscillating behavior. > > Signed-off-by: Benoit Canet > --- > block.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/block.c b/block.c > index 0a062c9..455d8b0 100644 > --- a/block.c > +++ b/block.c > @@ -3739,7 +3739,7 @@ static bool bdrv_exceed_iops_limits(BlockDriverState > *bs, bool is_write, > } > > /* Calc approx time to dispatch */ > -wait_time = (ios_base + 1) / iops_limit; > +wait_time = ios_base / iops_limit; > if (wait_time > elapsed_time) { > wait_time = wait_time - elapsed_time; > } else { I tried reproducing without your test case: dd if=/dev/vdb of=/dev/null bs=4096 iflag=direct I've pasted printfs below which reveals that wait time increases monotonically! In other words, dd is slowed down more and more as it runs: bs 0x7f56fe187a30 co 0x7f56fe211cb0 throttled for 1426 ms bs 0x7f56fe187a30 co 0x7f56fe211cb0 woke up from throttled_reqs after sleeping bs 0x7f56fe187a30 co 0x7f56fe211cb0 throttled for 1431 ms bs 0x7f56fe187a30 co 0x7f56fe211cb0 woke up from throttled_reqs after sleeping bs 0x7f56fe187a30 co 0x7f56fe211cb0 throttled for 1437 ms bs 0x7f56fe187a30 co 0x7f56fe211cb0 woke up from throttled_reqs after sleeping ... Killing dd and starting it again resets the accumulated delay (probably because we end the slice and state is cleared). This suggests workloads that are constantly at the I/O limit will experience creeping delay or the oscillations you found. After applying your patch I observed the opposite behavior: wait time decreases until it resets itself. Perhaps we're waiting less and less until we just finish the slice and all values reset: bs 0x7f2cd2c52a30 co 0x7f2cd2ce3910 throttled for 496 ms bs 0x7f2cd2c52a30 co 0x7f2cd2ce3910 woke up from throttled_reqs after sleeping bs 0x7f2cd2c52a30 co 0x7f2cd2ce3910 throttled for 489 ms bs 0x7f2cd2c52a30 co 0x7f2cd2ce3910 woke up from throttled_reqs after sleeping bs 0x7f2cd2c52a30 co 0x7f2cd2ce3910 throttled for 484 ms bs 0x7f2cd2c52a30 co 0x7f2cd2ce3910 woke up from throttled_reqs after sleeping bs 0x7f2cd2c52a30 co 0x7f2cd2ce3910 throttled for 480 ms bs 0x7f2cd2c52a30 co 0x7f2cd2ce3910 woke up from throttled_reqs after sleeping bs 0x7f2cd2c52a30 co 0x7f2cd2ce3910 throttled for 474 ms ... bs 0x7f2cd2c52a30 co 0x7f2cd2ce3910 throttled for 300 ms bs 0x7f2cd2c52a30 co 0x7f2cd2ce3910 woke up from throttled_reqs after sleeping bs 0x7f2cd2c52a30 co 0x7f2cd2ce3910 throttled for 299 ms bs 0x7f2cd2c52a30 co 0x7f2cd2ce3910 woke up from throttled_reqs after sleeping bs 0x7f2cd2c52a30 co 0x7f2cd2ce3910 throttled for 298 ms bs 0x7f2cd2c52a30 co 0x7f2cd2ce3910 woke up from throttled_reqs after sleeping bs 0x7f2cd2c52a30 co 0x7f2cd2ce3910 throttled for 494 ms I'm not confident that I understand the effects of your patch. Do you have an explanation for these results? More digging will probably be necessary to solve the underlying problem here. diff --git a/block.c b/block.c index 0a062c9..7a8c9e6 100644 --- a/block.c +++ b/block.c @@ -175,7 +175,9 @@ static void bdrv_io_limits_intercept(BlockDriverState *bs, int64_t wait_time = -1; if (!qemu_co_queue_empty(&bs->throttled_reqs)) { +fprintf(stderr, "bs %p co %p waiting for throttled_reqs\n", bs, qemu_coroutine_self()); qemu_co_queue_wait(&bs->throttled_reqs); +fprintf(stderr, "bs %p co %p woke up from throttled_reqs\n", bs, qemu_coroutine_self()); } /* In fact, we hope to keep each request's timing, in FIFO mode. The next @@ -188,7 +190,9 @@ static void bdrv_io_limits_intercept(BlockDriverState *bs, while (bdrv_exceed_io_limits(bs, nb_sectors, is_write, &wait_time)) { qemu_mod_timer(bs->block_timer, wait_time + qemu_get_clock_ns(vm_clock)); +fprintf(stderr, "bs %p co %p throttled for %"PRId64" ms\n", bs, qemu_coroutine_self(), wait_time qemu_co_queue_wait_insert_head(&bs->throttled_reqs); +fprintf(stderr, "bs %p co %p woke up from throttled_reqs after sleeping\n", bs, qemu_coroutine_s } qemu_co_queue_next(&bs->throttled_reqs);
Re: [Qemu-devel] [PATCH] qemu-sockets: Fix assertion failure
Am 20.03.2013 um 13:57 hat Luiz Capitulino geschrieben: > On Wed, 20 Mar 2013 09:39:34 +0100 > Kevin Wolf wrote: > > > Am 19.03.2013 um 21:34 hat Luiz Capitulino geschrieben: > > > inet_connect_addr() has two users: inet_connect_opts() and > > > wait_for_connect(), > > > with this patch both of them are now ignoring errors from > > > inet_connect_addr(). > > > > > > Suggested solution: refactor inet_connect_addr() to return an errno value. > > > Callers use error_set() when they want to report an error upward. > > > > Doesn't change the problem that you need to know when to set a return > > value != 0. So it doesn't help, but you'd lose some error information. > > My real point is that it's easier to check against errno to find out > the error cause (compared to using Error for that). You mean if the caller has to distinguish between different error codes? I think I would agree that avoiding Error can be a good way then if it doesn't lose error information. If we would lose information, using error classes other than generic would be acceptable, right? In the specific case, I don't think the callers make any difference and all errors are just errors, so this is mostly about the theory. Kevin
Re: [Qemu-devel] QOM-ify QemuConsoles ...
Hi Gerd, Am 20.03.2013 10:55, schrieb Gerd Hoffmann: > I think the next logical step ahead is to QOM-ify the QemuConsoles, so > we can link the QemuConsole to the thing actually backing it. For a > graphical console that would be the emlated graphic device. For a text > console it would be the serial line or monitor hooked up to it. > > With this in place we should be able to answer questions like "which > device backs this QemuConsole" by inspecting the object tree and handle > requests like "do a screendump of this device please". It will also be > useful to setup input routing: "pointer events from $this QemuConsole > should to $that virtual input device". > > Hints how to do that best? Pointers to sample code to look at? From a > brief look it seems we only QOM-ified emulated devices and not host-side > objects yet ... You could look at virtio-rng. TPM doesn't use QOM yet AFAIR. Cheers, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] QMP: TPM QMP and man page documentation updates
On 03/20/2013 08:32 AM, Markus Armbruster wrote: Corey Bryant writes: On 03/19/2013 03:26 AM, Markus Armbruster wrote: [Note cc: Anthony for QAPI schema expertise] Stefan Berger writes: On 03/18/2013 12:16 PM, Markus Armbruster wrote: Corey Bryant writes: Signed-off-by: Corey Bryant --- qemu-options.hx | 3 ++- qmp-commands.hx | 59 + 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/qemu-options.hx b/qemu-options.hx index 30fb85d..3b3cd0f 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2237,7 +2237,8 @@ Backend type must be: @option{passthrough}. The specific backend type will determine the applicable options. -The @code{-tpmdev} option requires a @code{-device} option. +The @code{-tpmdev} option creates the TPM backend and requires a +@code{-device} option that specifies the TPM frontend interface model. Options to each backend are described below. diff --git a/qmp-commands.hx b/qmp-commands.hx index b370060..4eda5ea 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -2721,18 +2721,77 @@ EQMP .mhandler.cmd_new = qmp_marshal_input_query_tpm, }, +SQMP +query-tpm +- + +Return information about the TPM device. + +Arguments: None + +Example: + +-> { "execute": "query-tpm" } +<- { "return": + [ + { "model": "tpm-tis", + "tpm-options": + { "type": "tpm-passthrough-options", + "data": + { "cancel-path": "/sys/class/misc/tpm0/device/cancel", + "path": "/dev/tpm0" + } + }, + "type": "passthrough", + "id": "tpm0" + } + ] + } + +EQMP + "tpm-options" is a discriminated union. How is its discriminator "type" (here: "tpm-passthrough-options") related to the outer "type" (here: "passthrough")? It gives you similar information twice. So there is a direct relationship between the two types. Awkward and undocumented. Relevant parts of qapi-schema.json: { 'enum': 'TpmType', 'data': [ 'passthrough' ] } { 'union': 'TpmTypeOptions', 'data': { 'tpm-passthrough-options' : 'TPMPassthroughOptions' } } { 'type': 'TPMInfo', 'data': {'id': 'str', 'model': 'TpmModel', 'type': 'TpmType', 'tpm-options': 'TpmTypeOptions' } } Type Netdev solves the same problem more elegantly: { 'union': 'NetClientOptions', 'data': { 'none': 'NetdevNoneOptions', 'nic': 'NetLegacyNicOptions', 'user': 'NetdevUserOptions', 'tap': 'NetdevTapOptions', 'socket': 'NetdevSocketOptions', 'vde': 'NetdevVdeOptions', 'dump': 'NetdevDumpOptions', 'bridge': 'NetdevBridgeOptions', 'hubport': 'NetdevHubPortOptions' } } { 'type': 'Netdev', 'data': { 'id': 'str', 'opts': 'NetClientOptions' } } Uses the union's discriminator. Straightforward. Following Netdev precedence, we get: { 'union': 'TpmTypeOptions', 'data': { 'passthrough' : 'TPMPassthroughOptions' } } { 'type': 'TPMInfo', 'data': {'id': 'str', 'model': 'TpmModel', 'opts': 'TpmTypeOptions' } } I can send a patch for this update if you'd like. Yes, please! Will do. Duplication of type is gone. No need for documentation. Since enum TpmType is used elsewhere, it still gets duplicated in the union's discriminator. Anthony, is there a way to name the implicit discriminator enum type for reference elsewhere in the schema? Are you saying it still gets duplicated with this fix? I'm not sure what you mean. A union in the schema implicitely defines an C enumeration type to be used for its discriminator. For instance, union TpmTypeOptions implicitely defines this one: typedef enum TpmTypeOptionsKind { TPM_TYPE_OPTIONS_KIND_TPM_PASSTHROUGH_OPTIONS = 0, TPM_TYPE_OPTIONS_KIND_MAX = 1, } TpmTypeOptionsKind; QAPI's discriminated union becomes a C struct containing the discriminator and the union of the members: struct TpmTypeOptions { TpmTypeOptionsKind kind; union { void *data; TPMPassthroughOptions * tpm_passthrough_options; }; }; Enum TpmType and type TpmInfo become: typedef enum TpmType { TPM_TYPE_PASSTHROUGH = 0, TPM_TYPE_MAX = 1, } TpmType; struct TPMInfo { char * id; TpmModel model; TpmType type; TpmTypeOptions * tpm_options; }; With the change I propose, TPM_TYPE_OPTIONS_KIND_TPM_PASSTHROUGH_OPTIONS becomes TPM_TYPE_OPTIONS_KIND_TPM_PASSTHROUGH, and TPMInfo member type goes away. Because TpmType is still used elsewhere, it doesn't go away, thus still duplicates TpmTypeOptionsKind. My question to Anthony wa
Re: [Qemu-devel] [PATCH 04/35] tpm: reorganize headers and split hardware part
On 03/18/2013 01:34 PM, Paolo Bonzini wrote: The TPM subsystem does not have a good front-end/back-end separation. I think it has very good front-end/back-end separation, but perhaps you mean the code could be moved around and better organized. However, we can at least try to split the user interface (tpm.c) from the implementation (hw/tpm). Here's a general break-down of front-end/back-end/general code tpm_tis.c = TIS front-end (the only front-end at this time) tpm_passthrough.c = Passthrough backend (the only back-end at this time) tpm.c = general code tpm_backend.c = backend thread pool functions (could use a better file name?) The patches makes tpm.c not include tpm_int.h; instead it moves more stuff to tpm_backend.h. Signed-off-by: Paolo Bonzini --- Makefile.objs | 2 +- default-configs/i386-softmmu.mak | 2 +- default-configs/x86_64-softmmu.mak | 2 +- hw/Makefile.objs | 1 + {tpm => hw/tpm}/Makefile.objs | 3 +- {tpm => hw/tpm}/tpm_backend.c | 14 ++ {tpm => hw/tpm}/tpm_int.h | 55 ++-- {tpm => hw/tpm}/tpm_passthrough.c | 0 {tpm => hw/tpm}/tpm_tis.c | 0 {tpm => hw/tpm}/tpm_tis.h | 5 {tpm => include/tpm}/tpm_backend.h | 57 ++ tpm/tpm.c => tpm.c | 18 ++-- 12 files changed, 80 insertions(+), 79 deletions(-) rename {tpm => hw/tpm}/Makefile.objs (61%) rename {tpm => hw/tpm}/tpm_backend.c (82%) rename {tpm => hw/tpm}/tpm_int.h (49%) rename {tpm => hw/tpm}/tpm_passthrough.c (100%) rename {tpm => hw/tpm}/tpm_tis.c (100%) rename {tpm => hw/tpm}/tpm_tis.h (94%) rename {tpm => include/tpm}/tpm_backend.h (50%) rename tpm/tpm.c => tpm.c (93%) diff --git a/Makefile.objs b/Makefile.objs index f99841c..ff3a6b3 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -74,7 +74,7 @@ common-obj-y += bt-host.o bt-vhci.o common-obj-y += dma-helpers.o common-obj-y += vl.o -common-obj-y += tpm/ +common-obj-y += tpm.o common-obj-$(CONFIG_SLIRP) += slirp/ diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak index 7d8908f..f70594d 100644 --- a/default-configs/i386-softmmu.mak +++ b/default-configs/i386-softmmu.mak @@ -26,4 +26,4 @@ CONFIG_HPET=y CONFIG_APPLESMC=y CONFIG_I8259=y CONFIG_PFLASH_CFI01=y -CONFIG_TPM_TIS=$(CONFIG_TPM) +CONFIG_TPM_TIS=y diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak index e87e644..66c4855 100644 --- a/default-configs/x86_64-softmmu.mak +++ b/default-configs/x86_64-softmmu.mak @@ -26,4 +26,4 @@ CONFIG_HPET=y CONFIG_APPLESMC=y CONFIG_I8259=y CONFIG_PFLASH_CFI01=y -CONFIG_TPM_TIS=$(CONFIG_TPM) +CONFIG_TPM_TIS=y diff --git a/hw/Makefile.objs b/hw/Makefile.objs index 09fea2c..5626292 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -25,6 +25,7 @@ devices-dirs-$(CONFIG_SOFTMMU) += scsi/ devices-dirs-$(CONFIG_SOFTMMU) += sd/ devices-dirs-$(CONFIG_SOFTMMU) += ssi/ devices-dirs-$(CONFIG_SOFTMMU) += timer/ +devices-dirs-$(CONFIG_TPM) += tpm/ devices-dirs-$(CONFIG_SOFTMMU) += usb/ devices-dirs-$(CONFIG_SOFTMMU) += virtio/ devices-dirs-$(CONFIG_SOFTMMU) += watchdog/ diff --git a/tpm/Makefile.objs b/hw/tpm/Makefile.objs similarity index 61% rename from tpm/Makefile.objs rename to hw/tpm/Makefile.objs index 366e4a7..8bbed7a 100644 --- a/tpm/Makefile.objs +++ b/hw/tpm/Makefile.objs @@ -1,4 +1,3 @@ -common-obj-y = tpm.o -common-obj-$(CONFIG_TPM) += tpm_backend.o +common-obj-y += tpm_backend.o common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o diff --git a/tpm/tpm_backend.c b/hw/tpm/tpm_backend.c similarity index 82% rename from tpm/tpm_backend.c rename to hw/tpm/tpm_backend.c index 4144ef7..31d833c 100644 --- a/tpm/tpm_backend.c +++ b/hw/tpm/tpm_backend.c @@ -56,3 +56,17 @@ void tpm_backend_thread_tpm_reset(TPMBackendThread *tbt, NULL); } } + +/* + * Write an error message in the given output buffer. + */ +void tpm_write_fatal_error_response(uint8_t *out, uint32_t out_len) +{ +if (out_len >= sizeof(struct tpm_resp_hdr)) { +struct tpm_resp_hdr *resp = (struct tpm_resp_hdr *)out; + +resp->tag = cpu_to_be16(TPM_TAG_RSP_COMMAND); +resp->len = cpu_to_be32(sizeof(struct tpm_resp_hdr)); +resp->errcode = cpu_to_be32(TPM_FAIL); +} +} I don't think moving this from tpm.c to tpm_backend.c helps anything. Maybe just renaming some of the files mentioned above might make the front-end vs back-end vs general code more intuitive. -- Regards, Corey Bryant diff --git a/tpm/tpm_int.h b/hw/tpm/tpm_int.h similarity index 49% rename from tpm/tpm_int.h rename to hw/tpm/tpm_int.h index f705643..d5f7bb8 100644 --- a/tpm/tpm_int.h +++ b/hw/tpm/tpm_int.h @@ -15,27 +15,8 @@ #include "exec/memory.h" #include "tpm/tpm_tis.h" -struct TPMDriverOps; -typedef struct
Re: [Qemu-devel] [PATCH] qemu-sockets: Fix assertion failure
On Wed, 20 Mar 2013 14:37:59 +0100 Kevin Wolf wrote: > Am 20.03.2013 um 13:57 hat Luiz Capitulino geschrieben: > > On Wed, 20 Mar 2013 09:39:34 +0100 > > Kevin Wolf wrote: > > > > > Am 19.03.2013 um 21:34 hat Luiz Capitulino geschrieben: > > > > inet_connect_addr() has two users: inet_connect_opts() and > > > > wait_for_connect(), > > > > with this patch both of them are now ignoring errors from > > > > inet_connect_addr(). > > > > > > > > Suggested solution: refactor inet_connect_addr() to return an errno > > > > value. > > > > Callers use error_set() when they want to report an error upward. > > > > > > Doesn't change the problem that you need to know when to set a return > > > value != 0. So it doesn't help, but you'd lose some error information. > > > > My real point is that it's easier to check against errno to find out > > the error cause (compared to using Error for that). > > You mean if the caller has to distinguish between different error codes? Yes. > I think I would agree that avoiding Error can be a good way then if it > doesn't lose error information. If we would lose information, using > error classes other than generic would be acceptable, right? Yes, I think so. I mean, even to me it's not entirely clear when new classes should be added. The rule I had in mind was that a new class should be added when a QMP client needs to distinguish a specific error. However, we're considering QEMU subsystems to be QMP clients, which (taken to an extreme) would lead us to our recent past where Error was trying to replace errno. Markus once wrote about where to set boundaries between errno and Error, but I don't remember if it was a private discussion or an email to the list. It's time to write this down in docs/.
Re: [Qemu-devel] [PATCH v2 05/15] qdev_prop_parse(): change return type to void
Luiz Capitulino writes: > On Thu, 7 Feb 2013 15:12:22 -0200 > Eduardo Habkost wrote: > >> On Tue, Feb 05, 2013 at 09:39:18PM +0100, Laszlo Ersek wrote: >> > Problems are now reported via Error only. >> > >> > Signed-off-by: Laszlo Ersek >> > --- >> > hw/qdev-properties.h |4 ++-- >> > hw/qdev-monitor.c|3 ++- >> > hw/qdev-properties.c | 14 ++ >> > 3 files changed, 10 insertions(+), 11 deletions(-) >> > >> > diff --git a/hw/qdev-properties.h b/hw/qdev-properties.h >> > index 0fe4030..43fd11a 100644 >> > --- a/hw/qdev-properties.h >> > +++ b/hw/qdev-properties.h >> > @@ -100,8 +100,8 @@ extern PropertyInfo qdev_prop_pci_host_devaddr; >> > >> > /* Set properties between creation and init. */ >> > void *qdev_get_prop_ptr(DeviceState *dev, Property *prop); >> > -int qdev_prop_parse(DeviceState *dev, const char *name, const char *value, >> > -Error **errp); >> > +void qdev_prop_parse(DeviceState *dev, const char *name, const char >> > *value, >> > + Error **errp); >> > void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value); >> > void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t >> > value); >> > void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t >> > value); >> > diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c >> > index 5eb1c8c..cf96046 100644 >> > --- a/hw/qdev-monitor.c >> > +++ b/hw/qdev-monitor.c >> > @@ -110,7 +110,8 @@ static int set_property(const char *name, const char >> > *value, void *opaque) >> > if (strcmp(name, "bus") == 0) >> > return 0; >> > >> > -if (qdev_prop_parse(dev, name, value, &err) == -1) { >> > +qdev_prop_parse(dev, name, value, &err); >> > +if (error_is_set(&err)) { >> >> You can use "if (err)" instead. I believe it is acceptable, as there's >> lots of (recently introduced) QEMU code using this style. > > Yes, people tend to use if (err) in new code. For me it honestly doesn't > matter much, although I wonder if we should have a more strict rule for this. I prefer plain "if (err)". >> > qerror_report_err(err); >> > error_free(err); >> > return -1; >> > diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c >> > index 8e3d014..d94909e 100644 >> > --- a/hw/qdev-properties.c >> > +++ b/hw/qdev-properties.c >> > @@ -835,8 +835,8 @@ void error_set_from_qdev_prop_error(Error **errp, int >> > ret, DeviceState *dev, >> > } >> > } >> > >> > -int qdev_prop_parse(DeviceState *dev, const char *name, const char *value, >> > -Error **errp) >> > +void qdev_prop_parse(DeviceState *dev, const char *name, const char >> > *value, >> > + Error **errp) >> > { >> > char *legacy_name; >> > Error *err = NULL; >> > @@ -849,11 +849,7 @@ int qdev_prop_parse(DeviceState *dev, const char >> > *name, const char *value, >> > } >> > g_free(legacy_name); >> > >> > -if (err) { >> > -error_propagate(errp, err); >> > -return -1; >> > -} >> > -return 0; >> > +error_propagate(errp, err); >> >> I didn't expect it to be valid to call error_propagate() if error is >> _not_ set. All cases of error_propagate() usage I have seen before first >> checked if error was set. But by looking at the implementation, it seems >> to be OK. Yes, it does the right thing. Weaker preconditions are good :) >> Maybe we should extend the error_propagate() documentation to mention it >> is OK to call error_propagate() even if error is unset? Makes sense. [...]
[Qemu-devel] [PATCH v4 00/10] virtio-scsi refactoring.
From: KONRAD Frederic This is the next part of virtio-refactoring. Basically it creates virtio-scsi device which extends virtio-device. Then a virtio-scsi can be connected on a virtio-bus. virtio-scsi-pci, virtio-scsi-s390x, virtio-scsi-ccw are created too, they extend respectively virtio-pci, virtio-s390-device, virtio-ccw-device and have a virtio-scsi. You can checkout my branch here: git://project.greensocs.com/qemu-virtio.git virtio-scsi-v4 Note that it is nearly the same series as virtio-blk refactoring. Though the 2nd and the 3rd steps are a virtio-scsi specific. I made basic tests (with linux guests) on: * qemu-system-i386 Changes v3 -> v4: * Added CCW device. * Fixed the configuration issue. Thanks, Fred KONRAD Frederic (10): virtio-scsi: don't use pointer for configuration. virtio-scsi: allocate cmd_vqs array separately. virtio-scsi: moving host_features from properties to transport properties. virtio-scsi: add the virtio-scsi device. virtio-scsi-pci: switch to new API. virtio-scsi-s390: switch to the new API. virtio-scsi-ccw: switch to new API virtio-scsi: cleanup: use QOM casts. virtio-scsi: cleanup: init and exit functions. virtio-scsi: cleanup: remove qdev field. hw/s390x/s390-virtio-bus.c | 30 --- hw/s390x/s390-virtio-bus.h | 11 +++- hw/s390x/virtio-ccw.c | 32 +++- hw/s390x/virtio-ccw.h | 12 - hw/virtio-pci.c| 119 ++- hw/virtio-pci.h| 15 +- hw/virtio-scsi.c | 122 ++--- hw/virtio-scsi.h | 21 8 files changed, 230 insertions(+), 132 deletions(-) -- 1.7.11.7
[Qemu-devel] [PATCH v4 02/10] virtio-scsi: allocate cmd_vqs array separately.
From: KONRAD Frederic Allocate/Free the cmd_vqs array separately to have a fixed size device. Signed-off-by: KONRAD Frederic --- hw/virtio-scsi.c | 7 --- hw/virtio-scsi.h | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c index 55191c5..08fcb80 100644 --- a/hw/virtio-scsi.c +++ b/hw/virtio-scsi.c @@ -690,12 +690,12 @@ VirtIODevice *virtio_scsi_init(DeviceState *dev, VirtIOSCSIConf *proxyconf) { VirtIOSCSI *s; static int virtio_scsi_id; -size_t sz; int i; -sz = sizeof(VirtIOSCSI) + proxyconf->num_queues * sizeof(VirtQueue *); s = (VirtIOSCSI *)virtio_common_init("virtio-scsi", VIRTIO_ID_SCSI, - sizeof(VirtIOSCSIConfig), sz); + sizeof(VirtIOSCSIConfig), + sizeof(VirtIOSCSI)); +s->cmd_vqs = g_malloc0(proxyconf->num_queues * sizeof(VirtQueue *)); s->qdev = dev; s->conf = *proxyconf; @@ -730,5 +730,6 @@ void virtio_scsi_exit(VirtIODevice *vdev) { VirtIOSCSI *s = (VirtIOSCSI *)vdev; unregister_savevm(s->qdev, "virtio-scsi", s); +g_free(s->cmd_vqs); virtio_cleanup(vdev); } diff --git a/hw/virtio-scsi.h b/hw/virtio-scsi.h index 6a0a95e..fb83b67 100644 --- a/hw/virtio-scsi.h +++ b/hw/virtio-scsi.h @@ -44,7 +44,7 @@ typedef struct VirtIOSCSI { bool events_dropped; VirtQueue *ctrl_vq; VirtQueue *event_vq; -VirtQueue *cmd_vqs[0]; +VirtQueue **cmd_vqs; } VirtIOSCSI; #define DEFINE_VIRTIO_SCSI_PROPERTIES(_state, _features_field, _conf_field) \ -- 1.7.11.7