Re: [Qemu-devel] [PATCH 3/3] qga: add guest-set-time command
On 01/08/2013 06:26 AM, Eric Blake wrote:
On 01/06/2013 03:07 AM, Lei Li wrote:
Signed-off-by: Lei Li
---
qga/commands-posix.c | 57 ++
qga/qapi-schema.json | 32
2 files changed, 89 insertions(+), 0 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 190199d..7fff49a 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -121,6 +121,63 @@ struct HostTimeInfo *qmp_guest_get_time(Error **errp)
return host_time;
}
+void qmp_guest_set_time(bool has_seconds, int64_t seconds,
+bool has_microseconds, int64_t microseconds,
+bool has_utc_offset, int64_t utc_offset, Error **errp)
+{
+int ret;
+int status;
+pid_t pid, rpid;
+struct timeval tv;
+HostTimeInfo *host_time;
+
+if ((!has_seconds) && (!has_microseconds) && (!has_utc_offset)) {
Is it really qemu style to parenthesize this much?
+host_time = get_host_time();
+if (!host_time) {
+error_set(errp, QERR_QGA_COMMAND_FAILED, "Failed to set guest
time");
+return;
+}
+tv.tv_sec = host_time->seconds;
+tv.tv_usec = host_time->microseconds;
+} else if (has_seconds && has_microseconds && has_utc_offset) {
+tv.tv_sec = (time_t) seconds + utc_offset;
You need to worry about overflow on hosts where time_t is 32-bits but
the user passed time using 64-bits (such as past the year 2038).
Likewise, it might be worth bounds-checking utc-offset to be at most 12
hours away from UTC (or is there a better bounds?).
+tv.tv_usec = (time_t) microseconds;
Likewise, you should range-validate that microseconds does not overflow
100 (or, if you take my suggestion about using nanoseconds, since
struct timespec is a bit more expressive, then bound things by
10, and properly round when converting to lower resolution
interfaces such as settimeofday()).
+} else {
+error_set(errp, QERR_INVALID_PARAMETER, "parameter missing");
That's a bit harsh. I'm thinking it might be nicer to support:
all three missing - grab time from the host
at least seconds present - populate any missing subseconds or utc_offset
as 0
seconds missing, but other fields present - error
making this look more like:
if (!has_seconds) {
if (has_subseconds || has_utc_offset) {
error_set();
} else {
use get_host_time();
}
} else {
tv.tv_sec = seconds + (has_utc_offset ? utc_offset : 0);
...
}
Good suggestions!
Yes, I know this is harsh. I will improve it in next version,
as well as the document.
+++ b/qga/qapi-schema.json
@@ -117,6 +117,38 @@
'returns': 'HostTimeInfo' }
##
+# @guest-set-time:
+#
+# Set guest time. If none arguments were given, will set
s/none/no/
+# host time to guest.
+#
+# Right now, when a guest is paused or migrated to a file
+# then loaded from that file, the guest OS has no idea that
+# there was a big gap in the time. Depending on how long
+# the gap was, NTP might not be able to resynchronize the
+# guest.
+#
+# This command tries to set guest time based on the information
+# from host or an absolute value given by management app, and
+# set the Hardware Clock to the current System Time. This
+# will make it easier for a guest to resynchronize without
+# waiting for NTP.
+#
+# @seconds: #optional "seconds" time.
+#
+# @microseconds: #optional "microseconds" time.
+#
+# @utc-offset: #optional utc offset.
If you like my above suggestions, this might be worth documenting that
@microseconds (or @nanoseconds) must not be provided unless @seconds is
present, and so on.
Same questions as in patch 1/3 - you need to document what @seconds is
relative to (presumably the Epoch of 1970-01-01), and what format
utc-offset takes. Based on this patch, it looks like you are using
utc-offset as the number of seconds difference, so one hour is
represented as 3600.
Sure. About the utc-offset format, I have replied to the previous patch.
It would be one-hour offset, and it's my mistake here...:(
--
Lei
[Qemu-devel] [Bug 1036645] Re: /usr/bin/xargs: rm: Argument list too long during make distclean in cross chroot
I see this with Ubuntu 12.04 inside of an armhf chroot. Running 'make deb-pkg' also exposes this issue ** Project changed: qemu => qemu-linaro (Ubuntu) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1036645 Title: /usr/bin/xargs: rm: Argument list too long during make distclean in cross chroot Status in “qemu-linaro” package in Ubuntu: New Bug description: I am building the Linux kernel in a cross chroot environment. When I run "make distclean", the following messages are emitted: [user@host:/home/work/linux]: make distclean /usr/bin/xargs: rm: Argument list too long make: *** [clean] Error 126 I create the cross chroot environment by making a copy of the root file system from an ARM system. I copy qemu-arm-static to /usr/bin and then chroot into the root file system. If I modify the make file to pass "-s 122880" to xargs, it fixes the problem. I have filed a bug report on xargs: http://savannah.gnu.org/bugs/index.php?37093 To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/qemu-linaro/+bug/1036645/+subscriptions
Re: [Qemu-devel] [PATCH 3/3] qga: add guest-set-time command
On 01/09/2013 09:40 PM, Luiz Capitulino wrote:
On Sun, 6 Jan 2013 18:07:00 +0800
Lei Li wrote:
Signed-off-by: Lei Li
---
qga/commands-posix.c | 57 ++
qga/qapi-schema.json | 32
2 files changed, 89 insertions(+), 0 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 190199d..7fff49a 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -121,6 +121,63 @@ struct HostTimeInfo *qmp_guest_get_time(Error **errp)
return host_time;
}
+void qmp_guest_set_time(bool has_seconds, int64_t seconds,
+bool has_microseconds, int64_t microseconds,
+bool has_utc_offset, int64_t utc_offset, Error **errp)
+{
+int ret;
+int status;
+pid_t pid, rpid;
+struct timeval tv;
+HostTimeInfo *host_time;
+
+if ((!has_seconds) && (!has_microseconds) && (!has_utc_offset)) {
+host_time = get_host_time();
+if (!host_time) {
+error_set(errp, QERR_QGA_COMMAND_FAILED, "Failed to set guest
time");
If you change get_host_time() to take an Error * argument, you can drop this.
ok.
+return;
+}
+tv.tv_sec = host_time->seconds;
+tv.tv_usec = host_time->microseconds;
+} else if (has_seconds && has_microseconds && has_utc_offset) {
+tv.tv_sec = (time_t) seconds + utc_offset;
+tv.tv_usec = (time_t) microseconds;
+} else {
+error_set(errp, QERR_INVALID_PARAMETER, "parameter missing");
Please, use error_setg() instead.
Sure.
+return;
+}
+
+ret = settimeofday(&tv, NULL);
+if (ret < 0) {
+error_set(errp, QERR_QGA_COMMAND_FAILED, strerror(errno));
Please, use error_setg_errno().
Yes.
+return;
+}
+
+/* Set the Hardware Clock to the current System Time. */
+pid = fork();
+if (pid == 0) {
+setsid();
+reopen_fd_to_null(0);
+reopen_fd_to_null(1);
+reopen_fd_to_null(2);
+
+execle("/sbin/hwclock", "hwclock", "-w", NULL, environ);
Honest question: is this really necessary? Can't we do whatever hwclock does?
I have thought about implementing this ourselves, and I did take a look
at the source code of hwclock. But looks like the implement ofthe
synchronization
for hardware clock and system clock is a little complicated, so I am not
sure if it's worth to have a try here.
+_exit(EXIT_FAILURE);
+} else if (pid < 0) {
+goto exit_err;
+}
+
+do {
+rpid = waitpid(pid, &status, 0);
+} while (rpid == -1 && errno == EINTR);
+if (rpid == pid && WIFEXITED(status) && !WEXITSTATUS(status)) {
+return;
+}
+
+exit_err:
+error_set(errp, QERR_UNDEFINED_ERROR);
+}
+
typedef struct GuestFileHandle {
uint64_t id;
FILE *fh;
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 4a8b93c..4649b55 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -117,6 +117,38 @@
'returns': 'HostTimeInfo' }
##
+# @guest-set-time:
+#
+# Set guest time. If none arguments were given, will set
+# host time to guest.
+#
+# Right now, when a guest is paused or migrated to a file
+# then loaded from that file, the guest OS has no idea that
+# there was a big gap in the time. Depending on how long
+# the gap was, NTP might not be able to resynchronize the
+# guest.
+#
+# This command tries to set guest time based on the information
+# from host or an absolute value given by management app, and
+# set the Hardware Clock to the current System Time. This
+# will make it easier for a guest to resynchronize without
+# waiting for NTP.
+#
+# @seconds: #optional "seconds" time.
+#
+# @microseconds: #optional "microseconds" time.
+#
+# @utc-offset: #optional utc offset.
+#
+# Returns: Nothing on success.
+#
+# Since: 1.4
+##
+{ 'command': 'guest-set-time',
+ 'data': { '*seconds': 'int', '*microseconds': 'int',
+'*utc-offset': 'int' } }
+
+##
# @GuestAgentCommandInfo:
#
# Information about guest agent commands.
--
Lei
Re: [Qemu-devel] [PATCH] sheepdog: implement direct write semantics
On 01/11/2013 03:52 PM, MORITA Kazutaka wrote: > At Thu, 10 Jan 2013 13:38:16 +0800, > Liu Yuan wrote: >> >> On 01/09/2013 11:10 PM, Paolo Bonzini wrote: >>> Il 09/01/2013 14:04, Liu Yuan ha scritto: >> 2 The upper layer software which relies on the 'cache=xxx' to choose >> cache mode will fail its assumption against new QEMU. > > Which assumptions do you mean? As far as I can say the behaviour hasn't > changed, except possibly for the performance. When users set 'cache=writethrough' to export only a writethrough cache to Guest, but with new QEMU, it will actually get a writeback cache as default. >>> >>> They get a writeback cache implementation-wise, but they get a >>> writethrough cache safety-wise. How the cache is implemented doesn't >>> matter, as long as it "looks like" a writethrough cache. >>> >> >>> In fact, consider a local disk that doesn't support FUA. In old QEMU, >>> images used to be opened with O_DSYNC and that splits each write into >>> WRITE+FLUSH, just like new QEMU. All that changes is _where_ the >>> flushes are created. Old QEMU changes it in the kernel, new QEMU >>> changes it in userspace. >>> We don't need to communicate to the guest. I think 'cache=xxx' means what kind of cache the users *expect* to export to Guest OS. So if cache=writethrough set, Guest OS couldn't turn it to writeback cache magically. This is like I bought a disk with 'writethrough' cache built-in, I didn't expect that it turned to be a disk with writeback cache under the hood which could possible lose data when power outage happened. >>> >>> It's not by magic. It's by explicitly requesting the disk to do this. >>> >>> Perhaps it's a bug that the cache mode is not reset when the machine is >>> reset. I haven't checked that, but it would be a valid complaint. >>> >> >> Ah I didn't get the current implementation right. I tried the 3.7 kernel >> and it works as expected (cache=writethrough result in a 'writethrough' >> cache in the guest). >> >> It looks fine to me to emulate writethrough as writeback + flush, since >> the profermance drop isn't big, though sheepdog itself support true >> writethrough cache (no flush). > > Can we drop the SD_FLAG_CMD_CACHE flag from sheepdog write requests > when bdrv_enable_write_cache() is false? Then the requests behave > like FUA writes and we can safely omit succeeding flush requests. > Let's first implement a emulated writethroughc cache and then look for a methond if we can play tricks to implement a true writethrough cache. This would bring complexity such as before we drop SD_FLAG_CMD_CACHE, we need to flush beforehand. And more, bdrv_enable_write_cache() is always true for now, I guess this need generic change block layer to get the writethrough/writeback hints. Thanks, Yuan
Re: [Qemu-devel] [Bug 108996 V2] hw/dma.c: Fix converting of ioport_register* to MemoryRegion
On Thu, Jan 10, 2013 at 06:00:25PM +0100, Andreas Färber wrote: > Am 10.01.2013 17:02, schrieb Stefan Hajnoczi: > > On Wed, Dec 19, 2012 at 12:09:21PM +, Julien Grall wrote: > >> The commit 582299336879504353e60c7937fbc70fea93f3da introduced a 1-shift > >> for > >> some offset in dma emulation. > >> > >> Before the previous commit, which converted ioport_register_* to > >> MemoryRegion, > >> the DMA controller registered 8 ioports with the following formula: > >> base + ((8 + i) << d->shift) where 0 <= i < 8 > >> When an IO occured within a Memory Region, DMA callback receives an offset > >> relative to the started address. Here the started address is: > >> base + (8 << d->shift). > >> The offset should be: (i << d->shift). After the shift is reverted, the > >> offset > >> are 0..7 not 1..8. > >> > >> Cc: 1089...@bugs.launchpad.net > >> Reviewed-by: Andreas Färber > >> Reported-by: Andreas Gustafsson > >> Signed-off-by: Julien Grall > >> --- > >> > >> Modification between V1 and V2: > >>* Modify the commit message to explain the problem. > >> > >> hw/dma.c | 22 +++--- > >> 1 file changed, 11 insertions(+), 11 deletions(-) > > > > This patch resolves "dma: unknown iport 0" warnings for my Windows 8 guest. > > > > Tested-by: Stefan Hajnoczi > > If you don't want to queue it for the trivial tree, I'll queue it > together with Hervé's conversions. Thanks, please queue it in your tree. Stefan
Re: [Qemu-devel] [Bug 108996 V2] hw/dma.c: Fix converting of ioport_register* to MemoryRegion
On Thu, Jan 10, 2013 at 06:00:25PM +0100, Andreas Färber wrote: > Am 10.01.2013 17:02, schrieb Stefan Hajnoczi: > > On Wed, Dec 19, 2012 at 12:09:21PM +, Julien Grall wrote: > >> The commit 582299336879504353e60c7937fbc70fea93f3da introduced a 1-shift > >> for > >> some offset in dma emulation. > >> > >> Before the previous commit, which converted ioport_register_* to > >> MemoryRegion, > >> the DMA controller registered 8 ioports with the following formula: > >> base + ((8 + i) << d->shift) where 0 <= i < 8 > >> When an IO occured within a Memory Region, DMA callback receives an offset > >> relative to the started address. Here the started address is: > >> base + (8 << d->shift). > >> The offset should be: (i << d->shift). After the shift is reverted, the > >> offset > >> are 0..7 not 1..8. > >> > >> Cc: 1089...@bugs.launchpad.net > >> Reviewed-by: Andreas Färber > >> Reported-by: Andreas Gustafsson > >> Signed-off-by: Julien Grall > >> --- > >> > >> Modification between V1 and V2: > >>* Modify the commit message to explain the problem. > >> > >> hw/dma.c | 22 +++--- > >> 1 file changed, 11 insertions(+), 11 deletions(-) > > > > This patch resolves "dma: unknown iport 0" warnings for my Windows 8 guest. > > > > Tested-by: Stefan Hajnoczi > > If you don't want to queue it for the trivial tree, I'll queue it > together with Hervé's conversions. Thanks, please take it through your tree. Stefan
Re: [Qemu-devel] [Qemu-trivial] [PATCH] configure: Fix comment (copy+paste bug)
On Sat, Jan 05, 2013 at 12:17:38PM +0100, Stefan Weil wrote: > Signed-off-by: Stefan Weil > --- > configure |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
Re: [Qemu-devel] [PATCH] qga/channel-posix.c: Explicitly include string.h
On Mon, Jan 07, 2013 at 05:29:55PM +, Peter Maydell wrote: > Explicitly include string.h to avoid warnings under MacOS X/clang > about implicit declarations of strerror() and strlen(). > > Signed-off-by: Peter Maydell > --- > I assume under Linux these are implicitly dragged in via one of the > other headers. > > qga/channel-posix.c | 1 + > 1 file changed, 1 insertion(+) Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
Re: [Qemu-devel] [Qemu-trivial] [PATCH] savevm: Remove MinGW specific code which is no longer needed
On Mon, Jan 07, 2013 at 10:20:27PM +0100, Stefan Weil wrote: > QEMU provides a portable function qemu_gettimeofday instead of > gettimeofday and also an implementation of localtime_r for MinGW. > > Signed-off-by: Stefan Weil > --- > savevm.c | 30 ++ > 1 file changed, 2 insertions(+), 28 deletions(-) Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
Re: [Qemu-devel] [Qemu-trivial] [PATCH] Replace remaining gmtime, localtime by gmtime_r, localtime_r
On Mon, Jan 07, 2013 at 11:08:13PM +0100, Stefan Weil wrote: > This allows removing of MinGW specific code and improves > reentrancy for POSIX hosts. > > Signed-off-by: Stefan Weil > --- > block.c | 10 -- > block/vvfat.c |4 > hw/omap1.c|2 +- > vl.c |8 +++- > 4 files changed, 4 insertions(+), 20 deletions(-) Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
Re: [Qemu-devel] [Qemu-trivial] [PATCH] Replace remaining gmtime, localtime by gmtime_r, localtime_r
On Mon, Jan 07, 2013 at 11:08:13PM +0100, Stefan Weil wrote:
> diff --git a/vl.c b/vl.c
> index f056c95..21e4a74 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -454,15 +454,13 @@ void qemu_get_timedate(struct tm *tm, int offset)
> ti += offset;
> if (rtc_date_offset == -1) {
> if (rtc_utc)
> -ret = gmtime(&ti);
> +ret = gmtime_r(&ti, tm);
> else
> -ret = localtime(&ti);
> +ret = localtime_r(&ti, tm);
> } else {
> ti -= rtc_date_offset;
> -ret = gmtime(&ti);
> +ret = gmtime_r(&ti, tm);
> }
> -
> -memcpy(tm, ret, sizeof(struct tm));
> }
BTW I fixed the compiler warning here, no need to respin:
vl.c: In function ‘qemu_get_timedate’:
vl.c:451:16: error: variable ‘ret’ set but not used
[-Werror=unused-but-set-variable]
Stefan
Re: [Qemu-devel] [PATCH 1/2] VFIO-AER: Vfio-pci driver changes for supporting AER
> -Original Message-
> From: Alex Williamson [mailto:alex.william...@redhat.com]
> Sent: Wednesday, January 09, 2013 11:05 AM
> To: Pandarathil, Vijaymohan R
> Cc: Bjorn Helgaas; Gleb Natapov; k...@vger.kernel.org; qemu-
> de...@nongnu.org; linux-...@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: Re: [PATCH 1/2] VFIO-AER: Vfio-pci driver changes for supporting
> AER
>
> On Wed, 2013-01-09 at 10:52 -0700, Alex Williamson wrote:
> > On Wed, 2013-01-09 at 06:26 +, Pandarathil, Vijaymohan R wrote:
> > > - New ioctl which is used to pass the eventfd that is signaled when
> > > an error occurs in the vfio_pci_device
> > >
> > > - Register pci_error_handler for the vfio_pci driver
> > >
> > > - When the device encounters an error, the error handler registered
> by
> > > the vfio_pci driver gets invoked by the AER infrastructure
> > >
> > > - In the error handler, signal the eventfd registered for the device.
> > >
> > > - This results in the qemu eventfd handler getting invoked and
> > > appropriate action taken for the guest.
> > >
> > > Signed-off-by: Vijay Mohan Pandarathil
> > > ---
> > > drivers/vfio/pci/vfio_pci.c | 29 +
> > > drivers/vfio/pci/vfio_pci_private.h | 1 +
> > > drivers/vfio/vfio.c | 8
> > > include/linux/vfio.h| 1 +
> > > include/uapi/linux/vfio.h | 9 +
> > > 5 files changed, 48 insertions(+)
> > >
> > > diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> > > index 6c11994..4ae9526 100644
> > > --- a/drivers/vfio/pci/vfio_pci.c
> > > +++ b/drivers/vfio/pci/vfio_pci.c
> > > @@ -207,6 +207,8 @@ static long vfio_pci_ioctl(void *device_data,
> > > if (vdev->reset_works)
> > > info.flags |= VFIO_DEVICE_FLAGS_RESET;
> > >
> > > + info.flags |= VFIO_DEVICE_FLAGS_AER_NOTIFY;
> > > +
> >
> > This appears to be a PCI specific flag, so the name should include
> > _PCI_. We also support non-PCIe devices and it seems like it would be
> > possible to not have AER support available, so shouldn't this be
> > conditional?
Will do that.
> >
> > > info.num_regions = VFIO_PCI_NUM_REGIONS;
> > > info.num_irqs = VFIO_PCI_NUM_IRQS;
> > >
> > > @@ -348,6 +350,19 @@ static long vfio_pci_ioctl(void *device_data,
> > >
> > > return ret;
> > >
> > > + } else if (cmd == VFIO_DEVICE_SET_ERRFD) {
> > > + int32_t fd = (int32_t)arg;
> > > +
> > > + if (fd < 0)
> > > + return -EINVAL;
> > > +
> > > + vdev->err_trigger = eventfd_ctx_fdget(fd);
> > > +
> > > + if (IS_ERR(vdev->err_trigger))
> > > + return PTR_ERR(vdev->err_trigger);
> > > +
> > > + return 0;
> > > +
> >
> > I'm not sure why we wouldn't describe this as just another interrupt
> > from the device and configure it via SET_IRQ. This ioctl has very
> > limited use and doesn't follow any of the conventions of all the other
> > vfio ioctls.
I thought this was a fairly simple ioctl to implement this way. Moreover,
there is no device interrupt involved. Let me know if you really want to
model it as a SET_IRQ ioctl.
> >
> > > } else if (cmd == VFIO_DEVICE_RESET)
> > > return vdev->reset_works ?
> > > pci_reset_function(vdev->pdev) : -EINVAL;
> > > @@ -527,11 +542,25 @@ static void vfio_pci_remove(struct pci_dev *pdev)
> > > kfree(vdev);
> > > }
> > >
> > > +static pci_ers_result_t vfio_err_detected(struct pci_dev *pdev,
> > > + pci_channel_state_t state)
> > > +{
> > > + struct vfio_pci_device *vdev = vfio_get_vdev(&pdev->dev);
> > > +
> > > + eventfd_signal(vdev->err_trigger, 1);
> > > + return PCI_ERS_RESULT_CAN_RECOVER;
> > > +}
> >
> > What if err_trigger hasn't been set?
Will fix that.
> >
> > > +
> > > +static const struct pci_error_handlers vfio_err_handlers = {
> > > + .error_detected = vfio_err_detected,
> > > +};
> > > +
> > > static struct pci_driver vfio_pci_driver = {
> > > .name = "vfio-pci",
> > > .id_table = NULL, /* only dynamic ids */
> > > .probe = vfio_pci_probe,
> > > .remove = vfio_pci_remove,
> > > + .err_handler= &vfio_err_handlers,
> > > };
> > >
> > > static void __exit vfio_pci_cleanup(void)
> > > diff --git a/drivers/vfio/pci/vfio_pci_private.h
> b/drivers/vfio/pci/vfio_pci_private.h
> > > index 611827c..daee62f 100644
> > > --- a/drivers/vfio/pci/vfio_pci_private.h
> > > +++ b/drivers/vfio/pci/vfio_pci_private.h
> > > @@ -55,6 +55,7 @@ struct vfio_pci_device {
> > > boolbardirty;
> > > struct pci_saved_state *pci_saved_state;
> > > atomic_trefcnt;
> > > + struct eventfd_ctx *err_trigger;
> > > };
> > >
> > > #define is_intx(vdev) (vdev->irq_type == VFIO_PCI_INTX_IRQ_INDEX)
> > > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> > > index 56097c6..5ed5a
Re: [Qemu-devel] [Qemu-trivial] [PATCH] Makefile: install the "acpi-dsdt.aml" and "q35-acpi-dsdt.aml" blobs too
On Tue, Jan 08, 2013 at 07:52:20PM +0100, Laszlo Ersek wrote: > The WARNING message from commit f7e4dd6c made me notice. > > Signed-off-by: Laszlo Ersek > --- > Makefile |1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/Makefile b/Makefile > index a7ac04b..f6626b5 100644 > --- a/Makefile > +++ b/Makefile > @@ -280,6 +280,7 @@ bepo > ifdef INSTALL_BLOBS > BLOBS=bios.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \ > vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin \ > +acpi-dsdt.aml q35-acpi-dsdt.aml \ > ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc \ > pxe-e1000.rom pxe-eepro100.rom pxe-ne2k_pci.rom \ > pxe-pcnet.rom pxe-rtl8139.rom pxe-virtio.rom \ I'm not familiar with the ACPI stuff and how we want to ship it. Gerd or Anthony: Please review this patch. Thanks, Stefan
[Qemu-devel] [PULL 0/4] q35: a bunch of little tweaks & fixes.
Hi, The biggest change is the machine type renaming patch. please pull, Gerd The following changes since commit a6308bc2224db238e72c570482717b68246a7ce0: Merge remote-tracking branch 'kraxel/build.1' into staging (2013-01-10 13:26:31 -0600) are available in the git repository at: git://git.kraxel.org/qemu q35.1 Gerd Hoffmann (3): q35: add ich9 intel hda controller q35: document chipset devices pc: rename machine types Laszlo Ersek (1): Makefile: install the "acpi-dsdt.aml" and "q35-acpi-dsdt.aml" blobs too Makefile |1 + docs/q35-chipset.cfg | 129 ++ hw/intel-hda.c | 41 +--- hw/pc_piix.c |8 ++-- hw/pc_q35.c |4 +- 5 files changed, 170 insertions(+), 13 deletions(-) create mode 100644 docs/q35-chipset.cfg
[Qemu-devel] [PATCH 1/4] q35: add ich9 intel hda controller
Signed-off-by: Gerd Hoffmann
---
hw/intel-hda.c | 41 ++---
1 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index 98ff936..eed1d38 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -1232,7 +1232,7 @@ static Property intel_hda_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
-static void intel_hda_class_init(ObjectClass *klass, void *data)
+static void intel_hda_class_init_common(ObjectClass *klass)
{
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
@@ -1240,20 +1240,46 @@ static void intel_hda_class_init(ObjectClass *klass,
void *data)
k->init = intel_hda_init;
k->exit = intel_hda_exit;
k->vendor_id = PCI_VENDOR_ID_INTEL;
-k->device_id = 0x2668;
-k->revision = 1;
k->class_id = PCI_CLASS_MULTIMEDIA_HD_AUDIO;
-dc->desc = "Intel HD Audio Controller";
dc->reset = intel_hda_reset;
dc->vmsd = &vmstate_intel_hda;
dc->props = intel_hda_properties;
}
-static TypeInfo intel_hda_info = {
+static void intel_hda_class_init_ich6(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+intel_hda_class_init_common(klass);
+k->device_id = 0x2668;
+k->revision = 1;
+dc->desc = "Intel HD Audio Controller (ich6)";
+}
+
+static void intel_hda_class_init_ich9(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+intel_hda_class_init_common(klass);
+k->device_id = 0x293e;
+k->revision = 3;
+dc->desc = "Intel HD Audio Controller (ich9)";
+}
+
+static TypeInfo intel_hda_info_ich6 = {
.name = "intel-hda",
.parent= TYPE_PCI_DEVICE,
.instance_size = sizeof(IntelHDAState),
-.class_init= intel_hda_class_init,
+.class_init= intel_hda_class_init_ich6,
+};
+
+static TypeInfo intel_hda_info_ich9 = {
+.name = "ich9-intel-hda",
+.parent= TYPE_PCI_DEVICE,
+.instance_size = sizeof(IntelHDAState),
+.class_init= intel_hda_class_init_ich9,
};
static void hda_codec_device_class_init(ObjectClass *klass, void *data)
@@ -1277,7 +1303,8 @@ static TypeInfo hda_codec_device_type_info = {
static void intel_hda_register_types(void)
{
type_register_static(&hda_codec_bus_info);
-type_register_static(&intel_hda_info);
+type_register_static(&intel_hda_info_ich6);
+type_register_static(&intel_hda_info_ich9);
type_register_static(&hda_codec_device_type_info);
}
--
1.7.1
[Qemu-devel] [PATCH 4/4] Makefile: install the "acpi-dsdt.aml" and "q35-acpi-dsdt.aml" blobs too
From: Laszlo Ersek The WARNING message from commit f7e4dd6c made me notice. Signed-off-by: Laszlo Ersek Signed-off-by: Gerd Hoffmann --- Makefile |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/Makefile b/Makefile index ee06448..0200bf3 100644 --- a/Makefile +++ b/Makefile @@ -280,6 +280,7 @@ bepo ifdef INSTALL_BLOBS BLOBS=bios.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \ vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin \ +acpi-dsdt.aml q35-acpi-dsdt.aml \ ppc_rom.bin openbios-sparc32 openbios-sparc64 openbios-ppc \ pxe-e1000.rom pxe-eepro100.rom pxe-ne2k_pci.rom \ pxe-pcnet.rom pxe-rtl8139.rom pxe-virtio.rom \ -- 1.7.1
[Qemu-devel] [PATCH 3/4] pc: rename machine types
Starting with release 1.4 we have a fully functional q35 machine type,
i.e. "qemu -M q35" JustWorks[tm]. Update machine type names to reflect
that:
* pc-1.4 becomes pc-i440fx-1.4
* q35-next becomes pc-q35-1.4
The pc-1.3 (+older) names are maintained for compatibility reasons.
For the same reason the "pc" and "q35" aliases are kept. pc-piix-1.4
continues to be the default machine type, again for compatibility
reasons.
Also updated the description (shown by "qemu -M ?") with host bridge
name, south bridge name and chipset release year.
Signed-off-by: Gerd Hoffmann
---
hw/pc_piix.c |8
hw/pc_q35.c |4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 2b3d58b..e630aea 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -282,10 +282,10 @@ static void pc_xen_hvm_init(QEMUMachineInitArgs *args)
}
#endif
-static QEMUMachine pc_machine_v1_4 = {
-.name = "pc-1.4",
+static QEMUMachine pc_i440fx_machine_v1_4 = {
+.name = "pc-i440fx-1.4",
.alias = "pc",
-.desc = "Standard PC",
+.desc = "Standard PC (i440FX + PIIX, 1996)",
.init = pc_init_pci_1_3,
.max_cpus = 255,
.is_default = 1,
@@ -646,7 +646,7 @@ static QEMUMachine xenfv_machine = {
static void pc_machine_init(void)
{
-qemu_register_machine(&pc_machine_v1_4);
+qemu_register_machine(&pc_i440fx_machine_v1_4);
qemu_register_machine(&pc_machine_v1_3);
qemu_register_machine(&pc_machine_v1_2);
qemu_register_machine(&pc_machine_v1_1);
diff --git a/hw/pc_q35.c b/hw/pc_q35.c
index ef540b6..52d9976 100644
--- a/hw/pc_q35.c
+++ b/hw/pc_q35.c
@@ -209,9 +209,9 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
}
static QEMUMachine pc_q35_machine = {
-.name = "q35-next",
+.name = "pc-q35-1.4",
.alias = "q35",
-.desc = "Q35 chipset PC",
+.desc = "Standard PC (Q35 + ICH9, 2009)",
.init = pc_q35_init,
.max_cpus = 255,
};
--
1.7.1
[Qemu-devel] [PATCH 2/4] q35: document chipset devices
Signed-off-by: Gerd Hoffmann --- docs/q35-chipset.cfg | 129 ++ 1 files changed, 129 insertions(+), 0 deletions(-) create mode 100644 docs/q35-chipset.cfg diff --git a/docs/q35-chipset.cfg b/docs/q35-chipset.cfg new file mode 100644 index 000..1b6efc0 --- /dev/null +++ b/docs/q35-chipset.cfg @@ -0,0 +1,129 @@ + +# +# qemu -M q35 creates a bare machine with just the very essential +# chipset devices being present: +# +# 00.0 - Host bridge +# 1f.0 - ISA bridge / LPC +# 1f.2 - SATA (AHCI) controller +# 1f.3 - SMBus controller +# +# This config file documents the other devices and how they are +# created. You can simply use "-readconfig $thisfile" to create +# them all. Here is a overview: +# +# 19.0 - Ethernet controller (not created, our e1000 emulation +# doesn't emulate the ich9 device). +# 1a.* - USB Controller #2 (ehci + uhci companions) +# 1b.0 - HD Audio Controller +# 1c.* - PCI Express Ports +# 1d.* - USB Controller #1 (ehci + uhci companions, +# "qemu -M q35 -usb" creates these too) +# 1e.0 - PCI Bridge +# + +[device "ich9-ehci-2"] + driver = "ich9-usb-ehci2" + multifunction = "on" + bus = "pcie.0" + addr = "1a.7" + +[device "ich9-uhci-4"] + driver = "ich9-usb-uhci4" + multifunction = "on" + bus = "pcie.0" + addr = "1a.0" + masterbus = "ich9-ehci-2.0" + firstport = "0" + +[device "ich9-uhci-5"] + driver = "ich9-usb-uhci5" + multifunction = "on" + bus = "pcie.0" + addr = "1a.1" + masterbus = "ich9-ehci-2.0" + firstport = "2" + +[device "ich9-uhci-6"] + driver = "ich9-usb-uhci6" + multifunction = "on" + bus = "pcie.0" + addr = "1a.2" + masterbus = "ich9-ehci-2.0" + firstport = "4" + + +[device "ich9-hda-audio"] + driver = "ich9-intel-hda" + bus = "pcie.0" + addr = "1b.0" + + +[device "ich9-pcie-port-1"] + driver = "ioh3420" + multifunction = "on" + bus = "pcie.0" + addr = "1c.0" + port = "1" + chassis = "1" + +[device "ich9-pcie-port-2"] + driver = "ioh3420" + multifunction = "on" + bus = "pcie.0" + addr = "1c.1" + port = "2" + chassis = "2" + +[device "ich9-pcie-port-3"] + driver = "ioh3420" + multifunction = "on" + bus = "pcie.0" + addr = "1c.2" + port = "3" + chassis = "3" + +[device "ich9-pcie-port-4"] + driver = "ioh3420" + multifunction = "on" + bus = "pcie.0" + addr = "1c.3" + port = "4" + chassis = "4" + + +[device "ich9-ehci-1"] + driver = "ich9-usb-ehci1" + multifunction = "on" + bus = "pcie.0" + addr = "1d.7" + +[device "ich9-uhci-1"] + driver = "ich9-usb-uhci1" + multifunction = "on" + bus = "pcie.0" + addr = "1d.0" + masterbus = "ich9-ehci-1.0" + firstport = "0" + +[device "ich9-uhci-2"] + driver = "ich9-usb-uhci2" + multifunction = "on" + bus = "pcie.0" + addr = "1d.1" + masterbus = "ich9-ehci-1.0" + firstport = "2" + +[device "ich9-uhci-3"] + driver = "ich9-usb-uhci3" + multifunction = "on" + bus = "pcie.0" + addr = "1d.2" + masterbus = "ich9-ehci-1.0" + firstport = "4" + + +[device "ich9-pci-bridge"] + driver = "i82801b11-bridge" + bus = "pcie.0" + addr = "1e.0" -- 1.7.1
Re: [Qemu-devel] [Qemu-trivial] [PATCH] Makefile: install the "acpi-dsdt.aml" and "q35-acpi-dsdt.aml" blobs too
On 01/11/13 09:48, Stefan Hajnoczi wrote: >> > +acpi-dsdt.aml q35-acpi-dsdt.aml \ > I'm not familiar with the ACPI stuff and how we want to ship it. > > Gerd or Anthony: Please review this patch. Patch is correct, just picked it up and stuffed into a q35 poll req. cheers, Gerd
Re: [Qemu-devel] [PATCH 2/2] QEMU-AER: Qemu changes to support AER for VFIO-PCI devices
> -Original Message-
> From: Blue Swirl [mailto:blauwir...@gmail.com]
> Sent: Wednesday, January 09, 2013 1:23 PM
> To: Pandarathil, Vijaymohan R
> Cc: Alex Williamson; Bjorn Helgaas; Gleb Natapov; linux-
> p...@vger.kernel.org; qemu-devel@nongnu.org; k...@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [Qemu-devel] [PATCH 2/2] QEMU-AER: Qemu changes to support AER
> for VFIO-PCI devices
>
> On Wed, Jan 9, 2013 at 6:26 AM, Pandarathil, Vijaymohan R
> wrote:
> > - Create eventfd per vfio device assigned to a guest and register
> an
> > event handler
> >
> > - This fd is passed to the vfio_pci driver through a new ioctl
> >
> > - When the device encounters an error, the eventfd is signaled
> > and the qemu eventfd handler gets invoked.
> >
> > - In the handler decide what action to take. Current action taken
> > is to terminate the guest.
> >
> > Signed-off-by: Vijay Mohan Pandarathil
> > ---
> > hw/vfio_pci.c | 56
> ++
> > linux-headers/linux/vfio.h | 9
> > 2 files changed, 65 insertions(+)
> >
> > diff --git a/hw/vfio_pci.c b/hw/vfio_pci.c
> > index 28c8303..9c3c28b 100644
> > --- a/hw/vfio_pci.c
> > +++ b/hw/vfio_pci.c
> > @@ -38,6 +38,7 @@
> > #include "qemu/error-report.h"
> > #include "qemu/queue.h"
> > #include "qemu/range.h"
> > +#include "sysemu/sysemu.h"
> >
> > /* #define DEBUG_VFIO */
> > #ifdef DEBUG_VFIO
> > @@ -130,6 +131,8 @@ typedef struct VFIODevice {
> > QLIST_ENTRY(VFIODevice) next;
> > struct VFIOGroup *group;
> > bool reset_works;
> > +EventNotifier errfd;
> > +__u32 dev_info_flags;
>
> QEMU is not kernel code, please use uint32_t.
As you saw later in the code, I was using the same type in the structure
copied from the kernel. Will change this to uint32_t.
Thanks
Vijay
>
> > } VFIODevice;
> >
> > typedef struct VFIOGroup {
> > @@ -1805,6 +1808,8 @@ static int vfio_get_device(VFIOGroup *group, const
> char *name, VFIODevice *vdev)
> > DPRINTF("Device %s flags: %u, regions: %u, irgs: %u\n", name,
> > dev_info.flags, dev_info.num_regions, dev_info.num_irqs);
> >
> > +vdev->dev_info_flags = dev_info.flags;
> > +
> > if (!(dev_info.flags & VFIO_DEVICE_FLAGS_PCI)) {
> > error_report("vfio: Um, this isn't a PCI device\n");
> > goto error;
> > @@ -1900,6 +1905,55 @@ static void vfio_put_device(VFIODevice *vdev)
> > }
> > }
> >
> > +static void vfio_errfd_handler(void *opaque)
> > +{
> > +VFIODevice *vdev = opaque;
> > +
> > +if (!event_notifier_test_and_clear(&vdev->errfd)) {
> > +return;
> > +}
> > +
> > +/*
> > + * TBD. Retrieve the error details and decide what action
> > + * needs to be taken. One of the actions could be to pass
> > + * the error to the guest and have the guest driver recover
> > + * the error. This requires that PCIe capabilities be
> > + * exposed to the guest. At present, we just terminate the
> > + * guest to contain the error.
> > + */
> > +error_report("%s(%04x:%02x:%02x.%x) "
> > +"Unrecoverable error detected... Terminating guest\n",
> > +__func__, vdev->host.domain, vdev->host.bus, vdev->host.slot,
> > +vdev->host.function);
> > +
> > +qemu_system_shutdown_request();
> > +return;
> > +}
> > +
> > +static void vfio_register_errfd(VFIODevice *vdev)
> > +{
> > +int32_t pfd;
> > +int ret;
> > +
> > +if (!(vdev->dev_info_flags & VFIO_DEVICE_FLAGS_AER_NOTIFY)) {
> > +error_report("vfio: Warning: Error notification not supported
> for the device\n");
> > +return;
> > +}
> > +if (event_notifier_init(&vdev->errfd, 0)) {
> > +error_report("vfio: Warning: Unable to init event notifier for
> error detection\n");
> > +return;
> > +}
> > +pfd = event_notifier_get_fd(&vdev->errfd);
> > +qemu_set_fd_handler(pfd, vfio_errfd_handler, NULL, vdev);
> > +
> > +ret = ioctl(vdev->fd, VFIO_DEVICE_SET_ERRFD, pfd);
> > +if (ret) {
> > +error_report("vfio: Warning: Failed to setup error fd: %d\n",
> ret);
> > +qemu_set_fd_handler(pfd, NULL, NULL, vdev);
> > +event_notifier_cleanup(&vdev->errfd);
> > +}
> > +return;
> > +}
> > static int vfio_initfn(PCIDevice *pdev)
> > {
> > VFIODevice *pvdev, *vdev = DO_UPCAST(VFIODevice, pdev, pdev);
> > @@ -2010,6 +2064,8 @@ static int vfio_initfn(PCIDevice *pdev)
> > }
> > }
> >
> > +vfio_register_errfd(vdev);
> > +
> > return 0;
> >
> > out_teardown:
> > diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
> > index 4758d1b..0ca4eeb 100644
> > --- a/linux-headers/linux/vfio.h
> > +++ b/linux-headers/linux/vfio.h
> > @@ -147,6 +147,7 @@ struct vfio_device_info {
> > __u32 flags;
> > #define VFIO_DEVICE_FLAGS_RESET(1 << 0)/* Device
> supports reset *
Re: [Qemu-devel] [PATCH 2/2] QEMU-AER: Qemu changes to support AER for VFIO-PCI devices
> -Original Message-
> From: Alex Williamson [mailto:alex.william...@redhat.com]
> Sent: Wednesday, January 09, 2013 10:08 AM
> To: Pandarathil, Vijaymohan R
> Cc: Bjorn Helgaas; Gleb Natapov; k...@vger.kernel.org; qemu-
> de...@nongnu.org; linux-...@vger.kernel.org; linux-ker...@vger.kernel.org
> Subject: Re: [PATCH 2/2] QEMU-AER: Qemu changes to support AER for VFIO-PCI
> devices
>
> On Wed, 2013-01-09 at 06:26 +, Pandarathil, Vijaymohan R wrote:
> > - Create eventfd per vfio device assigned to a guest and register an
> > event handler
> >
> > - This fd is passed to the vfio_pci driver through a new ioctl
> >
> > - When the device encounters an error, the eventfd is signaled
> > and the qemu eventfd handler gets invoked.
> >
> > - In the handler decide what action to take. Current action taken
> > is to terminate the guest.
> >
> > Signed-off-by: Vijay Mohan Pandarathil
> > ---
> > hw/vfio_pci.c | 56
> ++
> > linux-headers/linux/vfio.h | 9
> > 2 files changed, 65 insertions(+)
> >
> > diff --git a/hw/vfio_pci.c b/hw/vfio_pci.c
> > index 28c8303..9c3c28b 100644
> > --- a/hw/vfio_pci.c
> > +++ b/hw/vfio_pci.c
> > @@ -38,6 +38,7 @@
> > #include "qemu/error-report.h"
> > #include "qemu/queue.h"
> > #include "qemu/range.h"
> > +#include "sysemu/sysemu.h"
> >
> > /* #define DEBUG_VFIO */
> > #ifdef DEBUG_VFIO
> > @@ -130,6 +131,8 @@ typedef struct VFIODevice {
> > QLIST_ENTRY(VFIODevice) next;
> > struct VFIOGroup *group;
> > bool reset_works;
> > +EventNotifier errfd;
>
> Misleading name, it's an EventNotifier not an fd.
Will make the change.
>
> > +__u32 dev_info_flags;
> > } VFIODevice;
> >
> > typedef struct VFIOGroup {
> > @@ -1805,6 +1808,8 @@ static int vfio_get_device(VFIOGroup *group, const
> char *name, VFIODevice *vdev)
> > DPRINTF("Device %s flags: %u, regions: %u, irgs: %u\n", name,
> > dev_info.flags, dev_info.num_regions, dev_info.num_irqs);
> >
> > +vdev->dev_info_flags = dev_info.flags;
> > +
>
> We test the VFIO_DEVICE_FLAGS_RESET elsewhere by caching the bit into a
> variable, why not do that here too?
I was wondering if there was any specific reason to cache the bit into a
separate variable. Wouldn't a flags field which can contain the specific bit be
more apt ?
>
> > if (!(dev_info.flags & VFIO_DEVICE_FLAGS_PCI)) {
> > error_report("vfio: Um, this isn't a PCI device\n");
> > goto error;
> > @@ -1900,6 +1905,55 @@ static void vfio_put_device(VFIODevice *vdev)
> > }
> > }
> >
> > +static void vfio_errfd_handler(void *opaque)
> > +{
> > +VFIODevice *vdev = opaque;
> > +
> > +if (!event_notifier_test_and_clear(&vdev->errfd)) {
> > +return;
> > +}
> > +
> > +/*
> > + * TBD. Retrieve the error details and decide what action
> > + * needs to be taken. One of the actions could be to pass
> > + * the error to the guest and have the guest driver recover
> > + * the error. This requires that PCIe capabilities be
> > + * exposed to the guest. At present, we just terminate the
> > + * guest to contain the error.
> > + */
> > +error_report("%s(%04x:%02x:%02x.%x) "
> > +"Unrecoverable error detected... Terminating guest\n",
> > +__func__, vdev->host.domain, vdev->host.bus, vdev->host.slot,
> > +vdev->host.function);
> > +
> > +qemu_system_shutdown_request();
>
> I would have figured hw_error
Hw_error() is probably more appropriate. Will make the change.
>
> > +return;
> > +}
> > +
> > +static void vfio_register_errfd(VFIODevice *vdev)
> > +{
> > +int32_t pfd;
>
> "pfd" is used elsewhere in vfio as "Pointer to FD", this is just a fd.
Will change.
>
> > +int ret;
> > +
> > +if (!(vdev->dev_info_flags & VFIO_DEVICE_FLAGS_AER_NOTIFY)) {
> > +error_report("vfio: Warning: Error notification not supported
> for the device\n");
>
> This should probably just be a debug print.
Okay.
>
> > +return;
> > +}
> > +if (event_notifier_init(&vdev->errfd, 0)) {
> > +error_report("vfio: Warning: Unable to init event notifier for
> error detection\n");
> > +return;
> > +}
> > +pfd = event_notifier_get_fd(&vdev->errfd);
> > +qemu_set_fd_handler(pfd, vfio_errfd_handler, NULL, vdev);
> > +
> > +ret = ioctl(vdev->fd, VFIO_DEVICE_SET_ERRFD, pfd);
> > +if (ret) {
> > +error_report("vfio: Warning: Failed to setup error fd: %d\n",
> ret);
> > +qemu_set_fd_handler(pfd, NULL, NULL, vdev);
> > +event_notifier_cleanup(&vdev->errfd);
> > +}
> > +return;
> > +}
> > static int vfio_initfn(PCIDevice *pdev)
> > {
> > VFIODevice *pvdev, *vdev = DO_UPCAST(VFIODevice, pdev, pdev);
> > @@ -2010,6 +2064,8 @@ static int vfio_initfn(PCIDevice *pdev)
> > }
> > }
> >
> > +vfio_register_errfd(vdev);
> > +
>
Re: [Qemu-devel] [PATCH] sheepdog: implement direct write semantics
> bdrv_enable_write_cache() is always > true for now, I guess this need generic change block layer to get the > writethrough/writeback hints. That's not correct. Try hdparm with an IDE disk, or sysfs with a SCSI disk (recent kernels will also support sysfs with an IDE or virtio-blk disk) and you'll see that it changes. Paolo
Re: [Qemu-devel] [PATCH] sheepdog: implement direct write semantics
On 01/11/2013 05:00 PM, Paolo Bonzini wrote: > That's not correct. Try hdparm with an IDE disk, or sysfs with a SCSI > disk (recent kernels will also support sysfs with an IDE or virtio-blk > disk) and you'll see that it changes. Okay, at least at startup, even with cache=writehtough set, I saw bdrv_enable_write_cache() returns true from sd_open(). So you mean this will be reset later (after sd_open())? Thanks, Yuan
Re: [Qemu-devel] [PATCH V2 07/10] snapshot: qmp use new internal API for external snapshot transaction
On Fri, Jan 11, 2013 at 02:22:28PM +0800, Wenchao Xia wrote:
> 于 2013-1-10 20:41, Stefan Hajnoczi 写道:
> >On Thu, Jan 10, 2013 at 11:21:22AM +0800, Wenchao Xia wrote:
> >>于 2013-1-9 20:44, Stefan Hajnoczi 写道:
> >>>On Mon, Jan 07, 2013 at 03:28:06PM +0800, Wenchao Xia wrote:
> This patch switch to internal common API to take group external
> snapshots from qmp_transaction interface. qmp layer simply does
> a translation from user input.
>
> Signed-off-by: Wenchao Xia
> ---
> blockdev.c | 215
>
> 1 files changed, 87 insertions(+), 128 deletions(-)
> >>>
> >>>An internal API for snapshots is not necessary. qmp_transaction() is
> >>>already usable both from the monitor and C code.
> >>>
> >>>The QAPI code generator creates structs that can be accessed directly
> >>>from C. qmp_transaction(), BlockdevAction, and BlockdevActionList *is*
> >>>the snapshot API. It just doesn't support internal snapshots yet, which
> >>>is what you are trying to add.
> >>>
> >>>To add internal snapshot support, define a BlockdevInternalSnapshot type
> >>>in qapi-schema.json and add internal snapshot support in
> >>>qmp_transaction().
> >>>
> >>>qmp_transaction() was designed with this in mind from the beginning and
> >>>dispatches based on BlockdevAction->kind.
> >>>
> >>>The patch series will become much smaller while still adding internal
> >>>snapshot support.
> >>>
> >>>Stefan
> >>>
> >>
> >> As API, qmp_transaction have following disadvantages:
> >>1) interface is based on string not data type inside qemu, that means
> >>other function calling it result in: bdrv->string->bdrv
> >
> >Use bdrv_get_device_name(). You already need to fill in filename or
> >snapshot name strings. This is not a big disadvantage.
> >
> Yes, not a big disadvantage, but why not save string operation but
> use (bdrv*) as much as possible?
>
> what happens will be:
>
> hmp-snapshot
> |
> qmp-snapshot
> |-
> |
> qmp-transactionsavevm(may be other..)
> |--|
> |
> internal transaction layer
Saving the string operation is not worth duplicating the API.
> >>2) all capability are forced to be exposed.
> >
> >Is there something you cannot expose?
> >
> As other component in qemu can use it, some option may
> be used only in qemu not to user. For eg, vm-state-size.
When we hit a limitation of QAPI then it needs to be extended. I'm sure
there's a solution for splitting or hiding parts of the QAPI generated
API.
> >>3) need structure to record each transaction state, such as
> >>BlkTransactionStates. Extending it is equal to add an internal layer.
> >
> >I agree that extending it is equal coding effort to adding an internal
> >layer because you'll need to refactor qmp_transaction() a bit to really
> >support additional action types.
> >
> >But it's the right thing to do. Don't add unnecessary layers just
> >because writing new code is more fun than extending existing code.
> >
> If this layer is not added but depending only qmp_transaction, there
> will be many "if else" fragment. I have tried that and the code
> is awkful, this layer did not bring extra burden only make what
> happens inside qmp_transaction clearer, I did not add this layer just
> for fun.
>
>
> >> Actually I started up by use qmp_transaction as API, but soon
> >>found that work is almost done around BlkTransactionStates, so
> >>added a layer around it clearly.
The qmp_transaction() implementation can be changed, I'm not saying you
have to hack in more if statements. It's cleanest to introduce a
BdrvActionOps abstraction:
typedef struct BdrvActionOps BdrvActionOps;
typedef struct BdrvTransactionState {
const BdrvActionOps *ops;
QLIST_ENTRY(BdrvTransactionState);
} BdrvTransactionState;
struct BdrvActionOps {
int (*prepare)(BdrvTransactionState *s, ...);
int (*commit)(BdrvTransactionState *s, ...);
int (*rollback)(BdrvTransactionState *s, ...);
};
BdrvTransactionState *bdrv_transaction_create(BlockdevAction *action);
Then qmp_transaction() can be generic code that steps through the
transactions. This is similar to what your series does and I think it's
the right direction.
But please don't duplicate the qmp_transaction() and
BlockdevAction/BlockdevActionList APIs. In other words, change the
engine, not the whole car.
Stefan
[Qemu-devel] [PATCH V5 0/6] HMP: allow parsing for sub command
These patches enhance HMP to allow it parse 2nd level of commands, such as info sub command list, which means foldered command with parameter is possible now. V2: Follow the way supposed by Markus, which make the infrastructure knows there is possible a 2nd level of command exist, instead of a hack. In this way extention of command folder level is easy. Moved function declaration and better doc according to comments. Removed the patch about info snapshots, which will goto another serial. V3: Split out code moving patch. V4: Removed change of qmp_find_cmd(). Removed name change of monitor_parse_command(). v5: Eliminate 'info' in mhandler for that it have same format with 'cmd' and info is not a special case but a sub-command now. Split out patch that checking for space before check for sub-command. Better comments for monitor_parse_command(). Add parameter start for better error tips in sub-command case. Add comments about how sub_table and mhandler interact. Better commit message and tips that "info " show error now. Wenchao Xia (6): HMP: add QDict to info callback handler HMP: delete info handler HMP: add infrastructure for sub command HMP: filter out space before check of sub-command HMP: move define of mon_cmds HMP: add sub command table to info hmp-commands.hx |3 +- hmp.c | 36 hmp.h | 36 hw/i8259.c |4 +- hw/lm32_pic.c |4 +- hw/lm32_pic.h |4 +- hw/loader.c |2 +- hw/loader.h |3 +- hw/pc.h |4 +- hw/pcmcia.h |2 +- hw/qdev-monitor.c |4 +- hw/qdev-monitor.h |4 +- hw/sun4m.c |4 +- hw/sun4m.h |4 +- hw/usb.h|2 +- hw/usb/bus.c|2 +- hw/usb/host-bsd.c |2 +- hw/usb/host-linux.c |2 +- include/net/net.h |2 +- include/net/slirp.h |2 +- include/sysemu/sysemu.h |4 +- monitor.c | 202 -- net/net.c |2 +- net/slirp.c |2 +- savevm.c|2 +- vl.c|2 +- 26 files changed, 175 insertions(+), 165 deletions(-)
[Qemu-devel] [PATCH V5 1/6] HMP: add QDict to info callback handler
This patch change all info call back function to take
additional QDict * parameter, which allow those command
take parameter. Now it is set to NULL at default case.
Signed-off-by: Wenchao Xia
---
hmp.c | 36 ++--
hmp.h | 36 ++--
hw/i8259.c |4 ++--
hw/lm32_pic.c |4 ++--
hw/lm32_pic.h |4 ++--
hw/loader.c |2 +-
hw/loader.h |3 ++-
hw/pc.h |4 ++--
hw/pcmcia.h |2 +-
hw/qdev-monitor.c |4 ++--
hw/qdev-monitor.h |4 ++--
hw/sun4m.c |4 ++--
hw/sun4m.h |4 ++--
hw/usb.h|2 +-
hw/usb/bus.c|2 +-
hw/usb/host-bsd.c |2 +-
hw/usb/host-linux.c |2 +-
include/net/net.h |2 +-
include/net/slirp.h |2 +-
include/sysemu/sysemu.h |4 ++--
monitor.c | 32
net/net.c |2 +-
net/slirp.c |2 +-
savevm.c|2 +-
vl.c|2 +-
25 files changed, 84 insertions(+), 83 deletions(-)
diff --git a/hmp.c b/hmp.c
index 9e9e624..2465d9b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -31,7 +31,7 @@ static void hmp_handle_error(Monitor *mon, Error **errp)
}
}
-void hmp_info_name(Monitor *mon)
+void hmp_info_name(Monitor *mon, const QDict *qdict)
{
NameInfo *info;
@@ -42,7 +42,7 @@ void hmp_info_name(Monitor *mon)
qapi_free_NameInfo(info);
}
-void hmp_info_version(Monitor *mon)
+void hmp_info_version(Monitor *mon, const QDict *qdict)
{
VersionInfo *info;
@@ -55,7 +55,7 @@ void hmp_info_version(Monitor *mon)
qapi_free_VersionInfo(info);
}
-void hmp_info_kvm(Monitor *mon)
+void hmp_info_kvm(Monitor *mon, const QDict *qdict)
{
KvmInfo *info;
@@ -70,7 +70,7 @@ void hmp_info_kvm(Monitor *mon)
qapi_free_KvmInfo(info);
}
-void hmp_info_status(Monitor *mon)
+void hmp_info_status(Monitor *mon, const QDict *qdict)
{
StatusInfo *info;
@@ -89,7 +89,7 @@ void hmp_info_status(Monitor *mon)
qapi_free_StatusInfo(info);
}
-void hmp_info_uuid(Monitor *mon)
+void hmp_info_uuid(Monitor *mon, const QDict *qdict)
{
UuidInfo *info;
@@ -98,7 +98,7 @@ void hmp_info_uuid(Monitor *mon)
qapi_free_UuidInfo(info);
}
-void hmp_info_chardev(Monitor *mon)
+void hmp_info_chardev(Monitor *mon, const QDict *qdict)
{
ChardevInfoList *char_info, *info;
@@ -111,7 +111,7 @@ void hmp_info_chardev(Monitor *mon)
qapi_free_ChardevInfoList(char_info);
}
-void hmp_info_mice(Monitor *mon)
+void hmp_info_mice(Monitor *mon, const QDict *qdict)
{
MouseInfoList *mice_list, *mouse;
@@ -131,7 +131,7 @@ void hmp_info_mice(Monitor *mon)
qapi_free_MouseInfoList(mice_list);
}
-void hmp_info_migrate(Monitor *mon)
+void hmp_info_migrate(Monitor *mon, const QDict *qdict)
{
MigrationInfo *info;
MigrationCapabilityStatusList *caps, *cap;
@@ -209,7 +209,7 @@ void hmp_info_migrate(Monitor *mon)
qapi_free_MigrationCapabilityStatusList(caps);
}
-void hmp_info_migrate_capabilities(Monitor *mon)
+void hmp_info_migrate_capabilities(Monitor *mon, const QDict *qdict)
{
MigrationCapabilityStatusList *caps, *cap;
@@ -228,13 +228,13 @@ void hmp_info_migrate_capabilities(Monitor *mon)
qapi_free_MigrationCapabilityStatusList(caps);
}
-void hmp_info_migrate_cache_size(Monitor *mon)
+void hmp_info_migrate_cache_size(Monitor *mon, const QDict *qdict)
{
monitor_printf(mon, "xbzrel cache size: %" PRId64 " kbytes\n",
qmp_query_migrate_cache_size(NULL) >> 10);
}
-void hmp_info_cpus(Monitor *mon)
+void hmp_info_cpus(Monitor *mon, const QDict *qdict)
{
CpuInfoList *cpu_list, *cpu;
@@ -272,7 +272,7 @@ void hmp_info_cpus(Monitor *mon)
qapi_free_CpuInfoList(cpu_list);
}
-void hmp_info_block(Monitor *mon)
+void hmp_info_block(Monitor *mon, const QDict *qdict)
{
BlockInfoList *block_list, *info;
@@ -326,7 +326,7 @@ void hmp_info_block(Monitor *mon)
qapi_free_BlockInfoList(block_list);
}
-void hmp_info_blockstats(Monitor *mon)
+void hmp_info_blockstats(Monitor *mon, const QDict *qdict)
{
BlockStatsList *stats_list, *stats;
@@ -360,7 +360,7 @@ void hmp_info_blockstats(Monitor *mon)
qapi_free_BlockStatsList(stats_list);
}
-void hmp_info_vnc(Monitor *mon)
+void hmp_info_vnc(Monitor *mon, const QDict *qdict)
{
VncInfo *info;
Error *err = NULL;
@@ -406,7 +406,7 @@ out:
qapi_free_VncInfo(info);
}
-void hmp_info_spice(Monitor *mon)
+void hmp_info_spice(Monitor *mon, const QDict *qdict)
{
SpiceChannelList *chan;
SpiceInfo *info;
@@ -453,7 +453,7 @@ out:
qapi_free_SpiceInfo(info);
}
-void hmp_info_balloon(Monitor *mon)
+void hmp_info_balloon(Monitor *mon, const QDict *qdict)
{
BalloonInfo *info;
Error *err
[Qemu-devel] [PATCH V5 4/6] HMP: filter out space before check of sub-command
This fix the case when user input "@command ". Original
it will return NULL for monitor_parse_command(), now
it will return the @command related instance.
Signed-off-by: Wenchao Xia
---
monitor.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/monitor.c b/monitor.c
index 5435dc3..7b752a2 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3588,6 +3588,9 @@ static const mon_cmd_t *monitor_parse_command(Monitor
*mon,
if (cmd->sub_table != NULL) {
p1 = p;
/* check if user set additional command */
+while (qemu_isspace(*p1)) {
+p1++;
+}
if (*p1 == '\0') {
return cmd;
}
--
1.7.1
[Qemu-devel] [PATCH 3/6] qga/channel-posix.c: Explicitly include string.h
From: Peter Maydell Explicitly include string.h to avoid warnings under MacOS X/clang about implicit declarations of strerror() and strlen(). Signed-off-by: Peter Maydell Reviewed-by: Stefan Weil Signed-off-by: Stefan Hajnoczi --- qga/channel-posix.c | 1 + 1 file changed, 1 insertion(+) diff --git a/qga/channel-posix.c b/qga/channel-posix.c index d4fd628..ca9e4aa 100644 --- a/qga/channel-posix.c +++ b/qga/channel-posix.c @@ -4,6 +4,7 @@ #include #include #include +#include #include "qemu/osdep.h" #include "qemu/sockets.h" #include "qga/channel.h" -- 1.8.0.2
[Qemu-devel] [PATCH 5/6] Replace remaining gmtime, localtime by gmtime_r, localtime_r
From: Stefan Weil
This allows removing of MinGW specific code and improves
reentrancy for POSIX hosts.
[Removed unused ret variable in qemu_get_timedate() to fix warning:
vl.c: In function ‘qemu_get_timedate’:
vl.c:451:16: error: variable ‘ret’ set but not used
[-Werror=unused-but-set-variable]
-- Stefan Hajnoczi]
Signed-off-by: Stefan Weil
Signed-off-by: Stefan Hajnoczi
---
block.c | 10 --
block/vvfat.c | 4
hw/omap1.c| 2 +-
vl.c | 9 +++--
4 files changed, 4 insertions(+), 21 deletions(-)
diff --git a/block.c b/block.c
index 4e28c55..60873ea 100644
--- a/block.c
+++ b/block.c
@@ -3338,11 +3338,7 @@ char *get_human_readable_size(char *buf, int buf_size,
int64_t size)
char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn)
{
char buf1[128], date_buf[128], clock_buf[128];
-#ifdef _WIN32
-struct tm *ptm;
-#else
struct tm tm;
-#endif
time_t ti;
int64_t secs;
@@ -3352,15 +3348,9 @@ char *bdrv_snapshot_dump(char *buf, int buf_size,
QEMUSnapshotInfo *sn)
"ID", "TAG", "VM SIZE", "DATE", "VM CLOCK");
} else {
ti = sn->date_sec;
-#ifdef _WIN32
-ptm = localtime(&ti);
-strftime(date_buf, sizeof(date_buf),
- "%Y-%m-%d %H:%M:%S", ptm);
-#else
localtime_r(&ti, &tm);
strftime(date_buf, sizeof(date_buf),
"%Y-%m-%d %H:%M:%S", &tm);
-#endif
secs = sn->vm_clock_nsec / 10;
snprintf(clock_buf, sizeof(clock_buf),
"%02d:%02d:%02d.%03d",
diff --git a/block/vvfat.c b/block/vvfat.c
index 83706ce..06e6654 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -529,13 +529,9 @@ static inline uint8_t fat_chksum(const direntry_t* entry)
/* if return_time==0, this returns the fat_date, else the fat_time */
static uint16_t fat_datetime(time_t time,int return_time) {
struct tm* t;
-#ifdef _WIN32
-t=localtime(&time); /* this is not thread safe */
-#else
struct tm t1;
t = &t1;
localtime_r(&time,t);
-#endif
if(return_time)
return cpu_to_le16((t->tm_sec/2)|(t->tm_min<<5)|(t->tm_hour<<11));
return cpu_to_le16((t->tm_mday)|((t->tm_mon+1)<<5)|((t->tm_year-80)<<9));
diff --git a/hw/omap1.c b/hw/omap1.c
index 8536e96..e85f2e2 100644
--- a/hw/omap1.c
+++ b/hw/omap1.c
@@ -2830,7 +2830,7 @@ static void omap_rtc_tick(void *opaque)
s->round = 0;
}
-memcpy(&s->current_tm, localtime(&s->ti), sizeof(s->current_tm));
+localtime_r(&s->ti, &s->current_tm);
if ((s->interrupts & 0x08) && s->ti == s->alarm_ti) {
s->status |= 0x40;
diff --git a/vl.c b/vl.c
index f056c95..aed4182 100644
--- a/vl.c
+++ b/vl.c
@@ -448,21 +448,18 @@ StatusInfo *qmp_query_status(Error **errp)
void qemu_get_timedate(struct tm *tm, int offset)
{
time_t ti;
-struct tm *ret;
time(&ti);
ti += offset;
if (rtc_date_offset == -1) {
if (rtc_utc)
-ret = gmtime(&ti);
+gmtime_r(&ti, tm);
else
-ret = localtime(&ti);
+localtime_r(&ti, tm);
} else {
ti -= rtc_date_offset;
-ret = gmtime(&ti);
+gmtime_r(&ti, tm);
}
-
-memcpy(tm, ret, sizeof(struct tm));
}
int qemu_timedate_diff(struct tm *tm)
--
1.8.0.2
[Qemu-devel] [PATCH 6/6] hw/pc.c: Fix converting of ioport_register* to MemoryRegion
From: Julien Grall
The commit 258711 introduced MemoryRegion to replace ioport_region*
for ioport 80h and F0h.
A MemoryRegion needs to have both read and write callback otherwise a segfault
will occur when an access is made.
The previous behaviour of this both ioport is to return 0x.
So keep this behaviour.
Reported-by: Adam Lackorzynski
Signed-off-by: Julien Grall
Tested-by: Adam Lackorzynski
Signed-off-by: Stefan Hajnoczi
---
hw/pc.c | 12
1 file changed, 12 insertions(+)
diff --git a/hw/pc.c b/hw/pc.c
index df0c48e..90b1bf7 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -103,6 +103,11 @@ static void ioport80_write(void *opaque, hwaddr addr,
uint64_t data,
{
}
+static uint64_t ioport80_read(void *opaque, hwaddr addr, unsigned size)
+{
+return 0x;
+}
+
/* MSDOS compatibility mode FPU exception support */
static qemu_irq ferr_irq;
@@ -123,6 +128,11 @@ static void ioportF0_write(void *opaque, hwaddr addr,
uint64_t data,
qemu_irq_lower(ferr_irq);
}
+static uint64_t ioportF0_read(void *opaque, hwaddr addr, unsigned size)
+{
+return 0x;
+}
+
/* TSC handling */
uint64_t cpu_get_tsc(CPUX86State *env)
{
@@ -960,6 +970,7 @@ static void cpu_request_exit(void *opaque, int irq, int
level)
static const MemoryRegionOps ioport80_io_ops = {
.write = ioport80_write,
+.read = ioport80_read,
.endianness = DEVICE_NATIVE_ENDIAN,
.impl = {
.min_access_size = 1,
@@ -969,6 +980,7 @@ static const MemoryRegionOps ioport80_io_ops = {
static const MemoryRegionOps ioportF0_io_ops = {
.write = ioportF0_write,
+.read = ioportF0_read,
.endianness = DEVICE_NATIVE_ENDIAN,
.impl = {
.min_access_size = 1,
--
1.8.0.2
Re: [Qemu-devel] [Qemu-trivial] [PATCH] hw/pc.c: Fix converting of ioport_register* to MemoryRegion
On Wed, Jan 09, 2013 at 06:10:22PM +, Julien Grall wrote: > The commit 258711 introduced MemoryRegion to replace ioport_region* > for ioport 80h and F0h. > A MemoryRegion needs to have both read and write callback otherwise a segfault > will occur when an access is made. > > The previous behaviour of this both ioport is to return 0x. > So keep this behaviour. > > Reported-by: Adam Lackorzynski > Signed-off-by: Julien Grall > --- > hw/pc.c | 12 > 1 file changed, 12 insertions(+) Thanks, applied to the trivial patches tree: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
Re: [Qemu-devel] [PATCH v2] sheepdog: implement direct write semantics
Am 11.01.2013 08:35, schrieb MORITA Kazutaka: > At Thu, 10 Jan 2013 16:03:47 +0800, > Liu Yuan wrote: >> >> From: Liu Yuan >> >> Sheepdog supports both writeback/writethrough write but has not yet supported >> DIRECTIO semantics which bypass the cache completely even if Sheepdog daemon >> is >> set up with cache enabled. >> >> Suppose cache is enabled on Sheepdog daemon size, the new cache control is >> >> cache=writeback # enable the writeback semantics for write >> cache=writethrough # enable the emulated writethrough semantics for write >> cache=directsync # disable cache competely >> >> Guest WCE toggling on the run time to toggle writeback/writethrough is also >> supported. >> >> Cc: MORITA Kazutaka >> Cc: Kevin Wolf >> Cc: Stefan Hajnoczi >> Signed-off-by: Liu Yuan >> --- >> v2: adopt to current emulated writethrough cache setting >> >> block/sheepdog.c | 70 >> +++--- >> 1 file changed, 40 insertions(+), 30 deletions(-) > > Reviewed-by: MORITA Kazutaka Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH] reading files from qcow2-formated image disk for windows system
On Fri, Jan 11, 2013 at 03:27:52PM +0800, 马磊 wrote: > On Fri, Jan 11, 2013 at 2:28 PM, Wanlong Gao wrote: > > > On 01/11/2013 11:39 AM, 马磊 wrote: > > > > > > > > > On Thu, Jan 10, 2013 at 8:20 PM, Daniel P. Berrange > > > > berra...@redhat.com>> wrote: > > > > > > On Wed, Jan 09, 2013 at 09:37:54PM +, Blue Swirl wrote: > > > > On Wed, Jan 9, 2013 at 7:31 AM, 马磊 > aware@gmail.com>> wrote: > > > > > > > > > > > > > > >>> Hi, > > > > >>> The final effect is as follows: > > > > >>> > > > > >>> > > > > >>> [malei@xentest-4-1 Fri Dec 28 ~/honeypot/xen/xen-4.1.2]$ > > qemu-img-xen cat > > > > >>> -f /1/boot.ini ~/vm-check.img > > > > >>> [boot loader] > > > > >>> timeout=30 > > > > >>> default=multi(0)disk(0)rdisk(0)partition(1)\WINDOWS > > > > >>> [operating systems] > > > > >>> multi(0)disk(0)rdisk(0)partition(1)\WINDOWS="Microsoft Windows > > XP > > > > >>> Professional" /noexecute=optin /fastdetect > > > > >>> > > > > >>> [malei@xentest-4-1 Fri Dec 28 ~/honeypot/xen/xen-4.1.2]$ > > qemu-img-xen ls > > > > >>> -l -d /1/ ~/vm-check.img > > > > >>> 【name size(bytes) dir? date > > > > >>> create-time】 > > > > >>> AUTOEXEC.BAT 0file 2010-12-2217:30:37 > > > > >>> boot.ini 211file 2010-12-23 > >01:24:41 > > > > >>> bootfont.bin 322730file 2004-11-23 > > 20:00:00 > > > > >>> > > > > >>> > > > > >>> > > > > >>> As you see above, the patch add two sub-commands for > > qemu-img-xen：cat and > > > > >>> ls. > > > > >>> > > > > >>> For details in the patch, please check the attachment. > > > > >>> > > > > >>> > > > > > > > > > > Does anyone prefer this feature?! > > > > > > > > Nice feature, but this approach would just clutter QEMU and give > > only > > > > readonly FAT or NTFS support. I think a more generally useful > > approach > > > > would be to use NBD or iSCSI to export the block device data from > > the > > > > image file (qemu-nbd already exists) and then make a tool that uses > > > > some combination of NBD/iSCSI client, all GRUB file systems and > > FUSE > > > > or other user space methods to access the contents of the > > filesystem. > > > > Probably also UML with a simple guest agent could provide > > read/write > > > > access to any file system that Linux supports. > > > > > > The latter is what libguestfs already provides. It boots a Linux > > kernel > > > and mini initrd containing a guest agent, to provide APIs to do > > arbitrary > > > manipulation of guest OS images. > > > > > > The reason libguestfs used a linux guest was precisely to avoid > > having > > > to re-implement drivers for every filesystem in existance like this > > > patch is trying todo. > > > > > > I don't think QEMU wants to be in the business of maintaining > > filesystem > > > drivers, so I'd reject this proposed patch. > > > > > > Regards, > > > Daniel > > > -- > > > |: http://berrange.com -o- > > http://www.flickr.com/photos/dberrange/ :| > > > |: http://libvirt.org -o- > > http://virt-manager.org :| > > > |: http://autobuild.org -o- > > http://search.cpan.org/~danberr/ :| > > > |: http://entangle-photo.org -o- > > http://live.gnome.org/gtk-vnc :| > > > > > > > > > > > > This feature could be configured to be optional in make file > > configuration according to individual preference. > > > _In addition, the fat32 and ntfs filesystem driver will not change for a > > long time so it needs no much maintainence once implemented._ > > > > As Daniel and Stefan said, you can try to use libguestfs [libguestfs.org] > > and qemu-nbd. > > In libguestfs, we provide virt-cat, virt-ls, etc. And support all the disk > > type which QEMU supported. > > > > Thanks, > > Wanlong Gao > > > > > *I used libguest, it's startup takes too long to meet specific requirements > under some time-sensitive circumstance. * Then use qemu-nbd instead. It's fast and you can use all the file systems supported by your host (ext4, xfs, fat, ntfs, etc), as well as LVM or encrypted volumes. Stefan
Re: [Qemu-devel] [RFC PATCH] PIIX: reset the VM when the Reset Control Register's RCPU bit gets set
Hi,
On 01/09/13 22:01, Blue Swirl wrote:
> On Tue, Jan 8, 2013 at 9:44 PM, Laszlo Ersek wrote:
>> +static void i440fx_host_config_write(void *opaque, hwaddr addr,
>> + uint64_t val, unsigned len)
>> +{
>> +if (addr == 1 && len == 1) {
>> +if (val & 4) {
>> +qemu_system_reset_request();
>> +}
>> +return;
>> +}
>> +pci_host_conf_le_ops.write(opaque, addr, val, len);
>> +}
>> +
>> +static MemoryRegionOps i440fx_host_conf_ops = {
>> +.read = NULL,
>> +.write = i440fx_host_config_write,
>> +.endianness = DEVICE_LITTLE_ENDIAN
>> +};
>> +
>> static int i440fx_pcihost_initfn(SysBusDevice *dev)
>> {
>> PCIHostState *s = PCI_HOST_BRIDGE(dev);
>>
>> -memory_region_init_io(&s->conf_mem, &pci_host_conf_le_ops, s,
>> +i440fx_host_conf_ops.read = pci_host_conf_le_ops.read;
>
> It would be cleaner to introduce a new memory region (without this
> copying) which passes 0xcf8 and 0xcfc to standard PCI host but catches
> accesses to 0xcf9. This may mean that pci_host_config_{read,write}
> will need to be exposed.
Do you mean:
(1) introducing the new "i440fx_host_conf_ops" struct-of-funcptrs with
detached functions (that is, duplicating the guts of
pci_host_config_{read,write} and modifying them, and then registering
s->conf_mem with this "i440fx_host_conf_ops"; or
(2) leaving s->conf_mem as-is, and introducing a sub-region just for
port 0xcf9, with higher visibility priority?
(I don't feel confident about (2), and based on "docs/memory.txt" I
thought that overlapping regions had not been invented for this purpose.)
IOW, are you OK with the explicit offset + access-width based check,
just organized differently, or are you proposing a one-byte-wide subregion?
Thanks!
Laszlo
Re: [Qemu-devel] [PATCH] e1000: document ICS read behaviour
On Thu, Jan 10, 2013 at 02:03:39PM +0200, Michael S. Tsirkin wrote: > Add code comment to clarify the reason we set ICS with ICR: > the reason was previously undocumented and git > log (commit b1332393cdd7d023de8f1f8aa136ee7866a18968) > confused rather than clarified the comments. > Digging in the mailing list archives gives the real reason > https://lists.gnu.org/archive/html/qemu-devel/2009-06/msg00401.html > > Add code comment with an explanation supplied by Bill Paul. > > Signed-off-by: Michael S. Tsirkin > --- > hw/e1000.c | 10 ++ > 1 file changed, 10 insertions(+) Thanks, applied to the net tree: https://github.com/stefanha/qemu/commits/net Stefan
Re: [Qemu-devel] [PATCH] sheepdog: implement direct write semantics
Am 11.01.2013 08:52, schrieb MORITA Kazutaka: > At Thu, 10 Jan 2013 13:38:16 +0800, > Liu Yuan wrote: >> >> On 01/09/2013 11:10 PM, Paolo Bonzini wrote: >>> Il 09/01/2013 14:04, Liu Yuan ha scritto: >> 2 The upper layer software which relies on the 'cache=xxx' to choose >> cache mode will fail its assumption against new QEMU. > > Which assumptions do you mean? As far as I can say the behaviour hasn't > changed, except possibly for the performance. When users set 'cache=writethrough' to export only a writethrough cache to Guest, but with new QEMU, it will actually get a writeback cache as default. >>> >>> They get a writeback cache implementation-wise, but they get a >>> writethrough cache safety-wise. How the cache is implemented doesn't >>> matter, as long as it "looks like" a writethrough cache. >>> >> >>> In fact, consider a local disk that doesn't support FUA. In old QEMU, >>> images used to be opened with O_DSYNC and that splits each write into >>> WRITE+FLUSH, just like new QEMU. All that changes is _where_ the >>> flushes are created. Old QEMU changes it in the kernel, new QEMU >>> changes it in userspace. >>> We don't need to communicate to the guest. I think 'cache=xxx' means what kind of cache the users *expect* to export to Guest OS. So if cache=writethrough set, Guest OS couldn't turn it to writeback cache magically. This is like I bought a disk with 'writethrough' cache built-in, I didn't expect that it turned to be a disk with writeback cache under the hood which could possible lose data when power outage happened. >>> >>> It's not by magic. It's by explicitly requesting the disk to do this. >>> >>> Perhaps it's a bug that the cache mode is not reset when the machine is >>> reset. I haven't checked that, but it would be a valid complaint. >>> >> >> Ah I didn't get the current implementation right. I tried the 3.7 kernel >> and it works as expected (cache=writethrough result in a 'writethrough' >> cache in the guest). >> >> It looks fine to me to emulate writethrough as writeback + flush, since >> the profermance drop isn't big, though sheepdog itself support true >> writethrough cache (no flush). > > Can we drop the SD_FLAG_CMD_CACHE flag from sheepdog write requests > when bdrv_enable_write_cache() is false? Then the requests behave > like FUA writes and we can safely omit succeeding flush requests. First we would need to make sure that on a writeback -> writethrough switch a flush happens. But once this is implemented, I think your suggestion would work well. Kevin
Re: [Qemu-devel] [PATCH] block: do not probe zero-sized disks
On Thu, Jan 10, 2013 at 03:39:27PM +0100, Paolo Bonzini wrote:
> A blank CD or DVD is visible as a zero-sized disks. Probing such
> disks will lead to an EIO and a failure to start the VM. Treating
> them as raw is a better solution.
>
> Signed-off-by: Paolo Bonzini
> ---
> block.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block.c b/block.c
> index c05875f..b9da10e 100644
> --- a/block.c
> +++ b/block.c
> @@ -532,7 +532,7 @@ static int find_image_format(const char *filename,
> BlockDriver **pdrv)
> }
>
> /* Return the raw BlockDriver * to scsi-generic devices or empty drives
> */
> -if (bs->sg || !bdrv_is_inserted(bs)) {
> +if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) {
Looks good but I wouldn't understand the reason for the zero size check
when reading the code. Please add a comment so no one changes/removes
this code in the future.
Stefan
Re: [Qemu-devel] [PATCH] sheepdog: implement direct write semantics
- Messaggio originale - > Da: "Liu Yuan" > A: "Paolo Bonzini" > Cc: "Kevin Wolf" , "Stefan Hajnoczi" , > qemu-devel@nongnu.org, "MORITA Kazutaka" > > Inviato: Venerdì, 11 gennaio 2013 10:04:23 > Oggetto: Re: [Qemu-devel] [PATCH] sheepdog: implement direct write semantics > > On 01/11/2013 05:00 PM, Paolo Bonzini wrote: > > That's not correct. Try hdparm with an IDE disk, or sysfs with a > > SCSI disk (recent kernels will also support sysfs with an IDE or > > virtio-blk disk) and you'll see that it changes. > > Okay, at least at startup, even with cache=writehtough set, I saw > bdrv_enable_write_cache() returns true from sd_open(). So you mean > this will be reset later (after sd_open())? It is always true for the protocol. It is not always true for the format. This is correct. qcow2 can do some writes in writeback mode, but they will always be followed by a flush before the write is reported to the guest. It is a bit faster, and does not lose any correctness. Paolo
[Qemu-devel] [PATCH V5 2/6] HMP: delete info handler
Now cmd and info handler have same format, so delete info handler.
Signed-off-by: Wenchao Xia
---
monitor.c | 91 ++---
1 files changed, 45 insertions(+), 46 deletions(-)
diff --git a/monitor.c b/monitor.c
index c7b3014..359f333 100644
--- a/monitor.c
+++ b/monitor.c
@@ -123,7 +123,6 @@ typedef struct mon_cmd_t {
const char *help;
void (*user_print)(Monitor *mon, const QObject *data);
union {
-void (*info)(Monitor *mon, const QDict *qdict);
void (*cmd)(Monitor *mon, const QDict *qdict);
int (*cmd_new)(Monitor *mon, const QDict *params, QObject **ret_data);
int (*cmd_async)(Monitor *mon, const QDict *params,
@@ -824,7 +823,7 @@ static void do_info(Monitor *mon, const QDict *qdict)
goto help;
}
-cmd->mhandler.info(mon, NULL);
+cmd->mhandler.cmd(mon, NULL);
return;
help:
@@ -2434,63 +2433,63 @@ static mon_cmd_t info_cmds[] = {
.args_type = "",
.params = "",
.help = "show the version of QEMU",
-.mhandler.info = hmp_info_version,
+.mhandler.cmd = hmp_info_version,
},
{
.name = "network",
.args_type = "",
.params = "",
.help = "show the network state",
-.mhandler.info = do_info_network,
+.mhandler.cmd = do_info_network,
},
{
.name = "chardev",
.args_type = "",
.params = "",
.help = "show the character devices",
-.mhandler.info = hmp_info_chardev,
+.mhandler.cmd = hmp_info_chardev,
},
{
.name = "block",
.args_type = "",
.params = "",
.help = "show the block devices",
-.mhandler.info = hmp_info_block,
+.mhandler.cmd = hmp_info_block,
},
{
.name = "blockstats",
.args_type = "",
.params = "",
.help = "show block device statistics",
-.mhandler.info = hmp_info_blockstats,
+.mhandler.cmd = hmp_info_blockstats,
},
{
.name = "block-jobs",
.args_type = "",
.params = "",
.help = "show progress of ongoing block device operations",
-.mhandler.info = hmp_info_block_jobs,
+.mhandler.cmd = hmp_info_block_jobs,
},
{
.name = "registers",
.args_type = "",
.params = "",
.help = "show the cpu registers",
-.mhandler.info = do_info_registers,
+.mhandler.cmd = do_info_registers,
},
{
.name = "cpus",
.args_type = "",
.params = "",
.help = "show infos for each CPU",
-.mhandler.info = hmp_info_cpus,
+.mhandler.cmd = hmp_info_cpus,
},
{
.name = "history",
.args_type = "",
.params = "",
.help = "show the command line history",
-.mhandler.info = do_info_history,
+.mhandler.cmd = do_info_history,
},
#if defined(TARGET_I386) || defined(TARGET_PPC) || defined(TARGET_MIPS) || \
defined(TARGET_LM32) || (defined(TARGET_SPARC) && !defined(TARGET_SPARC64))
@@ -2500,11 +2499,11 @@ static mon_cmd_t info_cmds[] = {
.params = "",
.help = "show the interrupts statistics (if available)",
#ifdef TARGET_SPARC
-.mhandler.info = sun4m_irq_info,
+.mhandler.cmd = sun4m_irq_info,
#elif defined(TARGET_LM32)
-.mhandler.info = lm32_irq_info,
+.mhandler.cmd = lm32_irq_info,
#else
-.mhandler.info = irq_info,
+.mhandler.cmd = irq_info,
#endif
},
{
@@ -2513,11 +2512,11 @@ static mon_cmd_t info_cmds[] = {
.params = "",
.help = "show i8259 (PIC) state",
#ifdef TARGET_SPARC
-.mhandler.info = sun4m_pic_info,
+.mhandler.cmd = sun4m_pic_info,
#elif defined(TARGET_LM32)
-.mhandler.info = lm32_do_pic_info,
+.mhandler.cmd = lm32_do_pic_info,
#else
-.mhandler.info = pic_info,
+.mhandler.cmd = pic_info,
#endif
},
#endif
@@ -2526,7 +2525,7 @@ static mon_cmd_t info_cmds[] = {
.args_type = "",
.params = "",
.help = "show PCI info",
-.mhandler.info = hmp_info_pci,
+.mhandler.cmd = hmp_info_pci,
},
#if defined(TARGET_I386) || defined(TARGET_SH4) || defined(TARGET_SPARC) || \
defined(TARGET_PPC) || defined(TARGET_XTENSA)
@@ -2535,7 +2534,7 @@ static mon_cmd_t info_cmds[] = {
.args_type = "",
.params = "",
.help = "show virtual to physical memory mappings",
-.mhandler.info = tlb_info,
+.mhandler.cmd = tlb_info,
},
#endif
#if defined(TARGET_I386)
@@ -2544,7 +2543,7 @@ static mon_cmd_t info_cmds[] = {
.args_type = "",
.params = "",
[Qemu-devel] [PATCH 2/6] configure: Fix comment (copy+paste bug)
From: Stefan Weil Signed-off-by: Stefan Weil Signed-off-by: Stefan Hajnoczi --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index fe18ed2..148d5aa 100755 --- a/configure +++ b/configure @@ -2705,7 +2705,7 @@ if compile_prog "" "" ; then byteswap_h=yes fi -# Search for bswap_32 function +# Search for bswap32 function bswap_h=no cat > $TMPC << EOF #include -- 1.8.0.2
Re: [Qemu-devel] [PATCH] sheepdog: implement direct write semantics
On 01/11/2013 05:34 PM, Paolo Bonzini wrote: > It is always true for the protocol. It is not always true for the format. > So you mean bdrv_enable_write_cache() always return true for Sheepdog at any time? Thanks, Yuan
Re: [Qemu-devel] [PATCH] sheepdog: implement direct write semantics
> On 01/11/2013 05:34 PM, Paolo Bonzini wrote: > > It is always true for the protocol. It is not always true for the > > format. > > So you mean bdrv_enable_write_cache() always return true for Sheepdog > at any time? Yes, at least now. You could in principle have a format that calls bdrv_set_enable_write_cache(bs->file, ...), but nobody does it yet. Paolo
[Qemu-devel] [PATCH V5 6/6] HMP: add sub command table to info
Now info command takes a table of sub info commands,
and changed do_info() to do_info_help() to do help funtion
only.
Note that now "info " returns error instead
of list of info topics.
Signed-off-by: Wenchao Xia
---
hmp-commands.hx |3 ++-
monitor.c | 22 +-
2 files changed, 3 insertions(+), 22 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 010b8c9..87a411a 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1489,7 +1489,8 @@ ETEXI
.args_type = "item:s?",
.params = "[subcommand]",
.help = "show various information about the system state",
-.mhandler.cmd = do_info,
+.mhandler.cmd = do_info_help,
+.sub_table = info_cmds,
},
STEXI
diff --git a/monitor.c b/monitor.c
index f6cb659..d186532 100644
--- a/monitor.c
+++ b/monitor.c
@@ -810,28 +810,8 @@ static void user_async_cmd_handler(Monitor *mon, const
mon_cmd_t *cmd,
}
}
-static void do_info(Monitor *mon, const QDict *qdict)
+static void do_info_help(Monitor *mon, const QDict *qdict)
{
-const mon_cmd_t *cmd;
-const char *item = qdict_get_try_str(qdict, "item");
-
-if (!item) {
-goto help;
-}
-
-for (cmd = info_cmds; cmd->name != NULL; cmd++) {
-if (compare_cmd(item, cmd->name))
-break;
-}
-
-if (cmd->name == NULL) {
-goto help;
-}
-
-cmd->mhandler.cmd(mon, NULL);
-return;
-
-help:
help_cmd(mon, "info");
}
--
1.7.1
[Qemu-devel] [PATCH V5 3/6] HMP: add infrastructure for sub command
This patch make parsing of hmp command aware of that it may
have sub command. Also discard simple encapsulation function
monitor_find_command().
Signed-off-by: Wenchao Xia
---
monitor.c | 48 +---
1 files changed, 37 insertions(+), 11 deletions(-)
diff --git a/monitor.c b/monitor.c
index 359f333..5435dc3 100644
--- a/monitor.c
+++ b/monitor.c
@@ -129,6 +129,11 @@ typedef struct mon_cmd_t {
MonitorCompletion *cb, void *opaque);
} mhandler;
int flags;
+/* @sub_table is a list of 2nd level of commands. If it do not exist,
+ * mhandler should be used. If it exist, sub_table[?].mhandler should be
+ * used, and mhandler of 1st level plays the role of help function.
+ */
+struct mon_cmd_t *sub_table;
} mon_cmd_t;
/* file descriptors passed via SCM_RIGHTS */
@@ -3533,21 +3538,30 @@ static const mon_cmd_t *search_dispatch_table(const
mon_cmd_t *disp_table,
return NULL;
}
-static const mon_cmd_t *monitor_find_command(const char *cmdname)
-{
-return search_dispatch_table(mon_cmds, cmdname);
-}
-
static const mon_cmd_t *qmp_find_cmd(const char *cmdname)
{
return search_dispatch_table(qmp_cmds, cmdname);
}
+/*
+ * Parse @cmdline according to command table @table.
+ * If @cmdline is blank, return NULL.
+ * If it can't be parsed, report to @mon, and return NULL.
+ * Else, insert command arguments into @qdict, and return the command.
+ * If sub-command table exist, and if @cmdline contains addtional string for
+ * sub-command, this function will try search sub-command table. if no
+ * addtional string for sub-command exist, this function will return the found
+ * one in @table.
+ * Do not assume the returned command points into @table! It doesn't
+ * when the command is a sub-command.
+ */
static const mon_cmd_t *monitor_parse_command(Monitor *mon,
const char *cmdline,
+ int start,
+ mon_cmd_t *table,
QDict *qdict)
{
-const char *p, *typestr;
+const char *p, *p1, *typestr;
int c;
const mon_cmd_t *cmd;
char cmdname[256];
@@ -3555,20 +3569,32 @@ static const mon_cmd_t *monitor_parse_command(Monitor
*mon,
char *key;
#ifdef DEBUG
-monitor_printf(mon, "command='%s'\n", cmdline);
+monitor_printf(mon, "command='%s', start='%d'\n", cmdline, start);
#endif
/* extract the command name */
-p = get_command_name(cmdline, cmdname, sizeof(cmdname));
+p = get_command_name(cmdline + start, cmdname, sizeof(cmdname));
if (!p)
return NULL;
-cmd = monitor_find_command(cmdname);
+cmd = search_dispatch_table(table, cmdname);
if (!cmd) {
-monitor_printf(mon, "unknown command: '%s'\n", cmdname);
+monitor_printf(mon, "unknown command: '%.*s'\n",
+ (int)(p - cmdline), cmdline);
return NULL;
}
+/* search sub command */
+if (cmd->sub_table != NULL) {
+p1 = p;
+/* check if user set additional command */
+if (*p1 == '\0') {
+return cmd;
+}
+return monitor_parse_command(mon, cmdline, p1 - cmdline,
+ cmd->sub_table, qdict);
+}
+
/* parse the parameters */
typestr = cmd->args_type;
for(;;) {
@@ -3924,7 +3950,7 @@ static void handle_user_command(Monitor *mon, const char
*cmdline)
qdict = qdict_new();
-cmd = monitor_parse_command(mon, cmdline, qdict);
+cmd = monitor_parse_command(mon, cmdline, 0, mon_cmds, qdict);
if (!cmd)
goto out;
--
1.7.1
Re: [Qemu-devel] [RFC PATCH] virtio-net: introduce a new macaddr control
On Thu, Jan 10, 2013 at 10:51:57PM +0800, ak...@redhat.com wrote:
> @@ -349,6 +351,13 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t
> cmd,
> {
> struct virtio_net_ctrl_mac mac_data;
>
> +if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET && elem->out_num == 2) {
> +/* Set MAC address */
> +memcpy(n->mac, elem->out_sg[1].iov_base, elem->out_sg[1].iov_len);
We cannot trust the guest's iov_len, it could overflow n->mac.
[Qemu-devel] [PATCH 1/6] readline: avoid memcpy() of overlapping regions
From: Nickolai Zeldovich
memcpy() for overlapping regions is undefined behavior; use memmove()
instead in readline_hist_add().
[Keep tab characters since surrounding code still uses them -- Stefan]
Signed-off-by: Nickolai Zeldovich
Reviewed-by: Richard Henderson
Signed-off-by: Stefan Hajnoczi
---
readline.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/readline.c b/readline.c
index 5fc9643..a0c9638 100644
--- a/readline.c
+++ b/readline.c
@@ -248,8 +248,8 @@ static void readline_hist_add(ReadLineState *rs, const char
*cmdline)
if (idx == READLINE_MAX_CMDS) {
/* Need to get one free slot */
free(rs->history[0]);
- memcpy(rs->history, &rs->history[1],
- (READLINE_MAX_CMDS - 1) * sizeof(char *));
+ memmove(rs->history, &rs->history[1],
+ (READLINE_MAX_CMDS - 1) * sizeof(char *));
rs->history[READLINE_MAX_CMDS - 1] = NULL;
idx = READLINE_MAX_CMDS - 1;
}
--
1.8.0.2
[Qemu-devel] [PATCH 4/6] savevm: Remove MinGW specific code which is no longer needed
From: Stefan Weil
QEMU provides a portable function qemu_gettimeofday instead of
gettimeofday and also an implementation of localtime_r for MinGW.
Signed-off-by: Stefan Weil
Signed-off-by: Stefan Hajnoczi
---
savevm.c | 30 ++
1 file changed, 2 insertions(+), 28 deletions(-)
diff --git a/savevm.c b/savevm.c
index 529d60e..4e970ca 100644
--- a/savevm.c
+++ b/savevm.c
@@ -23,15 +23,6 @@
*/
#include "config-host.h"
-
-#ifndef _WIN32
-#include
-#endif
-
-#ifdef _WIN32
-#include
-#endif
-
#include "qemu-common.h"
#include "hw/hw.h"
#include "hw/qdev.h"
@@ -2093,13 +2084,8 @@ void do_savevm(Monitor *mon, const QDict *qdict)
QEMUFile *f;
int saved_vm_running;
uint64_t vm_state_size;
-#ifdef _WIN32
-struct _timeb tb;
-struct tm *ptm;
-#else
-struct timeval tv;
+qemu_timeval tv;
struct tm tm;
-#endif
const char *name = qdict_get_try_str(qdict, "name");
/* Verify if there is a device that doesn't support snapshots and is
writable */
@@ -2129,15 +2115,9 @@ void do_savevm(Monitor *mon, const QDict *qdict)
memset(sn, 0, sizeof(*sn));
/* fill auxiliary fields */
-#ifdef _WIN32
-_ftime(&tb);
-sn->date_sec = tb.time;
-sn->date_nsec = tb.millitm * 100;
-#else
-gettimeofday(&tv, NULL);
+qemu_gettimeofday(&tv);
sn->date_sec = tv.tv_sec;
sn->date_nsec = tv.tv_usec * 1000;
-#endif
sn->vm_clock_nsec = qemu_get_clock_ns(vm_clock);
if (name) {
@@ -2149,15 +2129,9 @@ void do_savevm(Monitor *mon, const QDict *qdict)
pstrcpy(sn->name, sizeof(sn->name), name);
}
} else {
-#ifdef _WIN32
-time_t t = tb.time;
-ptm = localtime(&t);
-strftime(sn->name, sizeof(sn->name), "vm-%Y%m%d%H%M%S", ptm);
-#else
/* cast below needed for OpenBSD where tv_sec is still 'long' */
localtime_r((const time_t *)&tv.tv_sec, &tm);
strftime(sn->name, sizeof(sn->name), "vm-%Y%m%d%H%M%S", &tm);
-#endif
}
/* Delete old snapshots of the same name */
--
1.8.0.2
[Qemu-devel] [PULL 0/6] Trivial patches for 5 to 11 January 2013
The following changes since commit 8e4a424b305e29dc0e454f52df3b35577f342975: Revert "virtio-pci: replace byte swap hack" (2013-01-06 18:30:17 +) are available in the git repository at: git://github.com/stefanha/qemu.git trivial-patches for you to fetch changes up to c02e1eac887b1b0aee7361b1fcf889e7d47fed9d: hw/pc.c: Fix converting of ioport_register* to MemoryRegion (2013-01-11 09:49:44 +0100) Julien Grall (1): hw/pc.c: Fix converting of ioport_register* to MemoryRegion Nickolai Zeldovich (1): readline: avoid memcpy() of overlapping regions Peter Maydell (1): qga/channel-posix.c: Explicitly include string.h Stefan Weil (3): configure: Fix comment (copy+paste bug) savevm: Remove MinGW specific code which is no longer needed Replace remaining gmtime, localtime by gmtime_r, localtime_r block.c | 10 -- block/vvfat.c | 4 configure | 2 +- hw/omap1.c | 2 +- hw/pc.c | 12 qga/channel-posix.c | 1 + readline.c | 4 ++-- savevm.c| 30 ++ vl.c| 9 +++-- 9 files changed, 22 insertions(+), 52 deletions(-) -- 1.8.0.2
Re: [Qemu-devel] [PATCH] Add libcacard/trace/generated-tracers.c to .gitignore
On Tue, Jan 08, 2013 at 01:28:02AM +0200, Alex Rozenman wrote: > Signed-off-by: Alex Rozenman > --- > .gitignore |1 + > 1 file changed, 1 insertion(+) Thanks, applied and will appear in the trivial patches tree next week: https://github.com/stefanha/qemu/commits/trivial-patches Stefan
[Qemu-devel] [PATCH V5 5/6] HMP: move define of mon_cmds
Because mon_cmds may use info_cmds, so adjust the declare sequence
of them.
Signed-off-by: Wenchao Xia
---
monitor.c | 12 ++--
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/monitor.c b/monitor.c
index 7b752a2..f6cb659 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2425,12 +2425,6 @@ int monitor_handle_fd_param(Monitor *mon, const char
*fdname)
return fd;
}
-/* mon_cmds and info_cmds would be sorted at runtime */
-static mon_cmd_t mon_cmds[] = {
-#include "hmp-commands.h"
-{ NULL, NULL, },
-};
-
/* Please update hmp-commands.hx when adding or changing commands */
static mon_cmd_t info_cmds[] = {
{
@@ -2744,6 +2738,12 @@ static mon_cmd_t info_cmds[] = {
},
};
+/* mon_cmds and info_cmds would be sorted at runtime */
+static mon_cmd_t mon_cmds[] = {
+#include "hmp-commands.h"
+{ NULL, NULL, },
+};
+
static const mon_cmd_t qmp_cmds[] = {
#include "qmp-commands-old.h"
{ /* NULL */ },
--
1.7.1
Re: [Qemu-devel] [PATCH 0/7 v2] KVM regsync
> -Original Message- > From: Jason J. Herne [mailto:jjhe...@linux.vnet.ibm.com] > Sent: Thursday, January 10, 2013 8:59 PM > To: ag...@suse.de; borntrae...@de.ibm.com; aligu...@us.ibm.com; > mtosa...@redhat.com; qemu-devel@nongnu.org; Bhushan Bharat-R65777; > jan.kis...@siemens.com > Subject: [PATCH 0/7 v2] KVM regsync > > Rework the method used to synchronize CPU registers between Qemu & KVM. This > patch set extends kvm_arch_put_registers() and > kvm_arch_get_registers() to take a register bitmap parameter. All existing > code > paths are updated to specify this new parameter. > > IMPORTANT NOTE: The PPC and i386 implementations are incomplete. > I am submitting this code at this time only to get a review on the > implementation of the existing code and to perhaps seek assistance with the > mentioned architectures. > > I am not sure who will finish the implementation of PPC/i386 yet. Due to the > fact that I am unfamiliar with these architectures at the register level and I > do not have test environments I would like to humbly request that a maintainer > of these architectures take a look at it. Or perhaps Bharat could handle the > PPC code? This would only leave I will handle the PPC code. -Bharat > i386 to worry about. If I cannot find someone to handle i386 I will look into > the feasibility of completing it myself. > > In order to complete the missing implementations, kvm_arch_get_registers and > kvm_arch_put_registers (and associated helper functions) will need to be > updated > to only sync the registers contained in the new bitmap argument. Also, each > set > of registers represented by one of the bits must be mutually exclusive with > respect to every other bit. if this is not the case then local register data > can be lost when kvm_arch_get_registers is called causing an old register > value > to overwrite a newer local value. > > -- > -- Jason J. Herne (jjhe...@linux.vnet.ibm.com) >
[Qemu-devel] [PATCH 0/6] Simple qemu-ga fixes and cleanups
Markus Armbruster (6): qemu-ga: Document intentional fall through in channel_event_cb() qemu-ga: Drop pointless lseek() from ga_open_pidfile() qemu-ga: Plug file descriptor leak on ga_open_pidfile() error path qemu-ga: Plug fd leak on ga_channel_listen_accept() error path qemu-ga: Plug fd leak on ga_channel_open() error paths qemu-ga: Handle errors uniformely in ga_channel_open() qga/channel-posix.c | 13 + qga/main.c | 5 - 2 files changed, 13 insertions(+), 5 deletions(-) -- 1.7.11.7
[Qemu-devel] [PATCH 3/6] qemu-ga: Plug file descriptor leak on ga_open_pidfile() error path
Spotted by Coverity. Also document why we keep it open on success. Signed-off-by: Markus Armbruster --- qga/main.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qga/main.c b/qga/main.c index 1a22d8d..db47427 100644 --- a/qga/main.c +++ b/qga/main.c @@ -286,10 +286,12 @@ static bool ga_open_pidfile(const char *pidfile) goto fail; } +/* keep pidfile open & locked forever */ return true; fail: unlink(pidfile); +close(pidfd); return false; } #else /* _WIN32 */ -- 1.7.11.7
[Qemu-devel] [PATCH 5/6] qemu-ga: Plug fd leak on ga_channel_open() error paths
Spotted by Coverity.
Signed-off-by: Markus Armbruster
---
qga/channel-posix.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index 5579185..b530808 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -153,6 +153,7 @@ static gboolean ga_channel_open(GAChannel *c, const gchar
*path, GAChannelMethod
ret = ga_channel_client_add(c, fd);
if (ret) {
g_critical("error adding channel to main loop");
+close(fd);
return false;
}
break;
--
1.7.11.7
[Qemu-devel] [PATCH 4/6] qemu-ga: Plug fd leak on ga_channel_listen_accept() error path
Spotted by Coverity.
Signed-off-by: Markus Armbruster
---
qga/channel-posix.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index d4fd628..5579185 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -45,6 +45,7 @@ static gboolean ga_channel_listen_accept(GIOChannel *channel,
ret = ga_channel_client_add(c, client_fd);
if (ret) {
g_warning("error setting up connection");
+close(client_fd);
goto out;
}
accepted = true;
--
1.7.11.7
[Qemu-devel] [PATCH V2 1/2] oslib-win32: add lock for gmtime_r()
Signed-off-by: Wenchao Xia
---
v2:
better comments and removed the code change gmtime() to gmtime_r().
---
oslib-win32.c |6 +-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/oslib-win32.c b/oslib-win32.c
index e7e283e..9a443da 100644
--- a/oslib-win32.c
+++ b/oslib-win32.c
@@ -32,6 +32,8 @@
#include "trace.h"
#include "qemu/sockets.h"
+static GStaticMutex time_lock = G_STATIC_MUTEX_INIT;
+
void *qemu_oom_check(void *ptr)
{
if (ptr == NULL) {
@@ -74,15 +76,17 @@ void qemu_vfree(void *ptr)
VirtualFree(ptr, 0, MEM_RELEASE);
}
-/* FIXME: add proper locking */
+/* FIXME: make it thread safe in MinGW, remove the lock in qemu. */
struct tm *gmtime_r(const time_t *timep, struct tm *result)
{
+g_static_mutex_lock(&time_lock);
struct tm *p = gmtime(timep);
memset(result, 0, sizeof(*result));
if (p) {
*result = *p;
p = result;
}
+g_static_mutex_unlock(&time_lock);
return p;
}
--
1.7.1
[Qemu-devel] [PATCH V2 2/2] oslib-win32: add lock for localtime_r()
Signed-off-by: Wenchao Xia
---
v2:
better comments and removed the code change localtime() to localtime_r().
---
oslib-win32.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/oslib-win32.c b/oslib-win32.c
index 9a443da..aa1268c 100644
--- a/oslib-win32.c
+++ b/oslib-win32.c
@@ -90,15 +90,17 @@ struct tm *gmtime_r(const time_t *timep, struct tm *result)
return p;
}
-/* FIXME: add proper locking */
+/* FIXME: make it thread safe in MinGW, remove the lock in qemu. */
struct tm *localtime_r(const time_t *timep, struct tm *result)
{
+g_static_mutex_lock(&time_lock);
struct tm *p = localtime(timep);
memset(result, 0, sizeof(*result));
if (p) {
*result = *p;
p = result;
}
+g_static_mutex_unlock(&time_lock);
return p;
}
--
1.7.1
Re: [Qemu-devel] [PATCH v2 06/10] chardev: add file chardev support to chardev-add (qmp)
>> ## +# @ChardevFile: > > Should you mention '@in: #optional' and '@out:' in any further > detail? Done. > Hmm; here you document ChardevFile as a separate type, but you > didn't document ChardevDummy in patch 4/10. As the name says it is just a dummy passed when the backend in question doesn't need any parameters. Therefore it is empty. Does it really need its own documentation stanza? cheers, Gerd
[Qemu-devel] [PATCH 1/6] qemu-ga: Document intentional fall through in channel_event_cb()
For clarity, and to hush up Coverity.
Signed-off-by: Markus Armbruster
---
qga/main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/qga/main.c b/qga/main.c
index a9b968c..47a6bea 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -605,6 +605,7 @@ static gboolean channel_event_cb(GIOCondition condition,
gpointer data)
if (!s->virtio) {
return false;
}
+/* fall through */
case G_IO_STATUS_AGAIN:
/* virtio causes us to spin here when no process is attached to
* host-side chardev. sleep a bit to mitigate this
--
1.7.11.7
[Qemu-devel] [PATCH 6/6] qemu-ga: Handle errors uniformely in ga_channel_open()
We detect errors in seven places. One reports with g_error(), which
calls abort(), the others report with g_critical(). Three of them
exit(), three return false.
Always report with g_critical(), and return false.
Signed-off-by: Markus Armbruster
---
qga/channel-posix.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index b530808..96274f5 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -140,14 +140,15 @@ static gboolean ga_channel_open(GAChannel *c, const gchar
*path, GAChannelMethod
);
if (fd == -1) {
g_critical("error opening channel: %s", strerror(errno));
-exit(EXIT_FAILURE);
+return false;
}
#ifdef CONFIG_SOLARIS
ret = ioctl(fd, I_SETSIG, S_OUTPUT | S_INPUT | S_HIPRI);
if (ret == -1) {
g_critical("error setting event mask for channel: %s",
strerror(errno));
-exit(EXIT_FAILURE);
+close(fd);
+return false;
}
#endif
ret = ga_channel_client_add(c, fd);
@@ -163,7 +164,7 @@ static gboolean ga_channel_open(GAChannel *c, const gchar
*path, GAChannelMethod
int fd = qemu_open(path, O_RDWR | O_NOCTTY | O_NONBLOCK);
if (fd == -1) {
g_critical("error opening channel: %s", strerror(errno));
-exit(EXIT_FAILURE);
+return false;
}
tcgetattr(fd, &tio);
/* set up serial port for non-canonical, dumb byte streaming */
@@ -183,7 +184,9 @@ static gboolean ga_channel_open(GAChannel *c, const gchar
*path, GAChannelMethod
tcsetattr(fd, TCSANOW, &tio);
ret = ga_channel_client_add(c, fd);
if (ret) {
-g_error("error adding channel to main loop");
+g_critical("error adding channel to main loop");
+close(fd);
+return false;
}
break;
}
--
1.7.11.7
Re: [Qemu-devel] [PATCH v2 07/10] chardev: add serial chardev support to chardev-add (qmp)
On 01/10/13 20:39, Eric Blake wrote: > On 01/10/2013 07:23 AM, Gerd Hoffmann wrote: >> Simliar to file, except that no separate in/out files are >> supported > > s/Simliar/Similar/ Fixed. >> +socket_set_nonblock(fd); > > Can this fail? No. cheers, Gerd
Re: [Qemu-devel] [PATCH v2 08/10] chardev: add parallel chardev support to chardev-add (qmp)
On 01/10/13 20:41, Eric Blake wrote:
> On 01/10/2013 07:23 AM, Gerd Hoffmann wrote:
>> Also alias the old parport name to parallel for -chardev.
>>
>> Signed-off-by: Gerd Hoffmann ---
>> qapi-schema.json |3 ++- qemu-char.c | 44
>> qemu-options.hx |
>> 5 - 3 files changed, 34 insertions(+), 18 deletions(-)
>>
>
>> ## -{ 'enum': 'ChardevPortKind', 'data': [ 'serial' ] } +{
>> 'enum': 'ChardevPortKind', 'data': [ 'serial', +
>> 'parallel' ] }
>>
>> { 'type': 'ChardevPort', 'data': { 'device' : 'str', 'type' :
>> 'ChardevPortKind'} }
>
> In patch 7, should the enum and type have separate documentation?
IMHO no, they belong together as ChardevPortKind is only used by
ChardevPort.
cheers,
Gerd
Re: [Qemu-devel] [PATCH 3/3 v2] Enable kvm emulated watchdog
Am 11.01.2013 um 07:42 schrieb Bhushan Bharat-R65777 :
>
>
>> -Original Message-
>> From: Alexander Graf [mailto:ag...@suse.de]
>> Sent: Thursday, January 10, 2013 9:07 PM
>> To: Bhushan Bharat-R65777
>> Cc: qemu-devel@nongnu.org; qemu-...@nongnu.org; Bhushan Bharat-R65777
>> Subject: Re: [PATCH 3/3 v2] Enable kvm emulated watchdog
>>
>>
>> On 28.12.2012, at 06:16, Bharat Bhushan wrote:
>>
>>> Enable the KVM emulated watchdog if KVM supports (use the capability
>>> enablement in watchdog handler). Also watchdog exit
>>> (KVM_EXIT_WATCHDOG) handling is added.
>>> Watchdog state machine is cleared whenever VM state changes to running.
>>> This is to handle the cases like return from debug halt etc.
>>>
>>> Signed-off-by: Bharat Bhushan
>>> ---
>>> v2:
>>> - access cap_* from target_ppc/kvm.c only.
>>> - Added wrapper functions in target_ppc/kvm.c for
>>> enable_watchdog and tsr_sregs synchronization.
>>> - Incorporated other Review comments
>>>
>>> hw/ppc.h |1 +
>>> hw/ppc_booke.c | 36 +++-
>>> target-ppc/kvm.c | 56
>>> ++
>>> target-ppc/kvm_ppc.h | 11 +
>>> 4 files changed, 103 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/hw/ppc.h b/hw/ppc.h
>>> index 2f3ea27..6ad9e1f 100644
>>> --- a/hw/ppc.h
>>> +++ b/hw/ppc.h
>>> @@ -90,3 +90,4 @@ enum {
>>>
>>> /* ppc_booke.c */
>>> void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t
>>> flags);
>>> +void ppc_booke_watchdog_clear_tsr(CPUPPCState *env);
>>> diff --git a/hw/ppc_booke.c b/hw/ppc_booke.c index 837a5b6..7273259
>>> 100644
>>> --- a/hw/ppc_booke.c
>>> +++ b/hw/ppc_booke.c
>>> @@ -28,7 +28,7 @@
>>> #include "nvram.h"
>>> #include "qemu-log.h"
>>> #include "loader.h"
>>> -
>>> +#include "kvm_ppc.h"
>>>
>>> /* Timer Control Register */
>>>
>>> @@ -203,6 +203,11 @@ static void booke_wdt_cb(void *opaque)
>>> booke_timer->wdt_timer); }
>>>
>>> +void ppc_booke_watchdog_clear_tsr(CPUPPCState *env) {
>>> +env->spr[SPR_BOOKE_TSR] &= ~(TSR_ENW | TSR_WIS | TSR_WRS_MASK); }
>>> +
>>> void store_booke_tsr(CPUPPCState *env, target_ulong val) {
>>>env->spr[SPR_BOOKE_TSR] &= ~val;
>>> @@ -241,10 +246,27 @@ static void ppc_booke_timer_reset_handle(void *opaque)
>>>booke_update_irq(env);
>>> }
>>>
>>> +static void cpu_state_change_handler(void *opaque, int running,
>>> +RunState state) {
>>
>> Needs a comment when this happens
>>
>>> +CPUPPCState *env = opaque;
>>> +
>>> +if (!running) {
>>> +return;
>>> +}
>>> +
>>> +/*
>>> + * Clear watchdog interrupt condition by clearing TSR.
>>> + */
>>> +ppc_booke_watchdog_clear_tsr(env);
>>> +
>>> +kvmppc_synch_sregs_tsr(env);
>>
>> kvmppc_sync_tsr. Also please add the sync to store_booke_tsr(). Then here,
>> you
>> can just do
>>
>> store_booke_tsr(TSR_ENW | TSR_WIS | TSR_WRS_MASK);
>>
>>> +}
>>> +
>>> void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t
>>> flags) {
>>>ppc_tb_t *tb_env;
>>>booke_timer_t *booke_timer;
>>> +int ret = 0;
>>>
>>>tb_env = g_malloc0(sizeof(ppc_tb_t));
>>>booke_timer = g_malloc0(sizeof(booke_timer_t)); @@ -262,5 +284,17
>>> @@ void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t
>>> flags)
>>>booke_timer->wdt_timer =
>>>qemu_new_timer_ns(vm_clock, &booke_wdt_cb, env);
>>>
>>> +ret = kvmppc_booke_watchdog_enable(env);
>>> +
>>> +if (ret) {
>>> +/* TODO: Start the QEMU emulated watchdog if not running on KVM.
>>> + * Also start the QEMU emulated watchdog if KVM does not support
>>> + * emulated watchdog or somehow it is not enabled (supported but
>>> + * not enabled is though some bug and requires debugging :)).
>>> + */
>>> +}
>>> +
>>> +qemu_add_vm_change_state_handler(cpu_state_change_handler, env);
>>> +
>>>qemu_register_reset(ppc_booke_timer_reset_handle, env); } diff
>>> --git a/target-ppc/kvm.c b/target-ppc/kvm.c index 3f5df57..6828afa
>>> 100644
>>> --- a/target-ppc/kvm.c
>>> +++ b/target-ppc/kvm.c
>>> @@ -32,10 +32,12 @@
>>> #include "device_tree.h"
>>> #include "hw/sysbus.h"
>>> #include "hw/spapr.h"
>>> +#include "hw/watchdog.h"
>>>
>>> #include "hw/sysbus.h"
>>> #include "hw/spapr.h"
>>> #include "hw/spapr_vio.h"
>>> +#include "hw/ppc.h"
>>>
>>> //#define DEBUG_KVM
>>>
>>> @@ -61,6 +63,7 @@ static int cap_ppc_smt; static int cap_ppc_rma;
>>> static int cap_spapr_tce; static int cap_hior;
>>> +static int cap_ppc_watchdog;
>>>
>>> /* XXX We have a race condition where we actually have a level triggered
>>> * interrupt, but the infrastructure can't expose that yet, so the guest
>>> @@ -90,6 +93,7 @@ int kvm_arch_init(KVMState *s)
>>>cap_ppc_rma = kvm_check_extension(s, KVM_CAP_PPC_RMA);
>>>cap_spapr_tce = kvm_check_extension(s, KVM_CAP_SPAPR_TCE);
>>>cap_hior = kvm_check_extension(s, KVM_CAP_PPC_HIOR);
>
[Qemu-devel] [PATCH 2/6] qemu-ga: Drop pointless lseek() from ga_open_pidfile()
After open(), the file offset is already zero, and neither lockf() nor
ftruncate() change it.
Signed-off-by: Markus Armbruster
---
qga/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qga/main.c b/qga/main.c
index 47a6bea..1a22d8d 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -276,7 +276,7 @@ static bool ga_open_pidfile(const char *pidfile)
return false;
}
-if (ftruncate(pidfd, 0) || lseek(pidfd, 0, SEEK_SET)) {
+if (ftruncate(pidfd, 0)) {
g_critical("Failed to truncate pid file");
goto fail;
}
--
1.7.11.7
Re: [Qemu-devel] [PATCH 1/3] qga: add support to get host time
On Fri, 11 Jan 2013 15:19:35 +0800 Lei Li wrote: > >> ## > >> +# @HostTimeInfo > > I'm a bit confused, why do you call it HostTimeInfo if this runs > > in the guest? > > I call it HostTimeInfo because it contains the host time information. qemu-ga runs in the guest, so it's actually the guest time. > But seems that all of you don't like this 'HostTimeInfo', 'TimeInfo' > might be better? Yes, looks better for me.
[Qemu-devel] [PATCH v3 05/10] chardev: add hmp hotplug commands
Add chardev-add and chardev-remove commands to the human monitor.
chardev-add accepts the same syntax as -chardev, chardev-remove
expects a chardev id.
Signed-off-by: Gerd Hoffmann
---
hmp-commands.hx | 32
hmp.c | 23 +++
hmp.h |2 ++
3 files changed, 57 insertions(+), 0 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 010b8c9..67569ef 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1485,6 +1485,38 @@ passed since 1970, i.e. unix epoch.
ETEXI
{
+.name = "chardev-add",
+.args_type = "args:s",
+.params = "args",
+.help = "add chardev",
+.mhandler.cmd = hmp_chardev_add,
+},
+
+STEXI
+@item chardev_add args
+@findex chardev_add
+
+chardev_add accepts the same parameters as the -chardev command line switch.
+
+ETEXI
+
+{
+.name = "chardev-remove",
+.args_type = "id:s",
+.params = "id",
+.help = "remove chardev",
+.mhandler.cmd = hmp_chardev_remove,
+},
+
+STEXI
+@item chardev_remove id
+@findex chardev_remove
+
+Removes the chardev @var{id}.
+
+ETEXI
+
+{
.name = "info",
.args_type = "item:s?",
.params = "[subcommand]",
diff --git a/hmp.c b/hmp.c
index 9e9e624..68929b4 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1336,3 +1336,26 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict
*qdict)
qmp_nbd_server_stop(&errp);
hmp_handle_error(mon, &errp);
}
+
+void hmp_chardev_add(Monitor *mon, const QDict *qdict)
+{
+const char *args = qdict_get_str(qdict, "args");
+Error *err = NULL;
+QemuOpts *opts;
+
+opts = qemu_opts_parse(qemu_find_opts("chardev"), args, 1);
+if (opts == NULL) {
+error_setg(&err, "Parsing chardev args failed\n");
+} else {
+qemu_chr_new_from_opts(opts, NULL, &err);
+}
+hmp_handle_error(mon, &err);
+}
+
+void hmp_chardev_remove(Monitor *mon, const QDict *qdict)
+{
+Error *local_err = NULL;
+
+qmp_chardev_remove(qdict_get_str(qdict, "id"), &local_err);
+hmp_handle_error(mon, &local_err);
+}
diff --git a/hmp.h b/hmp.h
index 21f3e05..700fbdc 100644
--- a/hmp.h
+++ b/hmp.h
@@ -80,5 +80,7 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict);
void hmp_nbd_server_start(Monitor *mon, const QDict *qdict);
void hmp_nbd_server_add(Monitor *mon, const QDict *qdict);
void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
+void hmp_chardev_add(Monitor *mon, const QDict *qdict);
+void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
#endif
--
1.7.1
[Qemu-devel] [PATCH v3 06/10] chardev: add file chardev support to chardev-add (qmp)
Add support for file chardevs. Output file is mandatory,
input file is optional.
Signed-off-by: Gerd Hoffmann
---
qapi-schema.json | 16 +-
qemu-char.c | 61 ++
qmp-commands.hx |8 ++-
3 files changed, 83 insertions(+), 2 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 462511e..c70c118 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3019,6 +3019,19 @@
{ 'command': 'nbd-server-stop' }
##
+# @ChardevFile:
+#
+# Configuration info for file chardevs.
+#
+# @in: #optional The name of the input file
+# @out: The name of the output file
+#
+# Since: 1.4
+##
+{ 'type': 'ChardevFile', 'data': { '*in' : 'str',
+ 'out' : 'str' } }
+
+##
# @ChardevBackend:
#
# Configuration info for the new chardev backend.
@@ -3027,7 +3040,8 @@
##
{ 'type': 'ChardevDummy', 'data': { } }
-{ 'union': 'ChardevBackend', 'data': { 'null' : 'ChardevDummy' } }
+{ 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile',
+ 'null' : 'ChardevDummy' } }
##
# @ChardevReturn:
diff --git a/qemu-char.c b/qemu-char.c
index dd4acf5..b8b5f16 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2938,6 +2938,64 @@ CharDriverState *qemu_char_get_next_serial(void)
return serial_hds[next_serial++];
}
+#ifdef _WIN32
+
+static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
+{
+HANDLE out;
+
+if (file->in) {
+error_setg(errp, "input file not supported");
+return NULL;
+}
+
+out = CreateFile(file->out, GENERIC_WRITE, FILE_SHARE_READ, NULL,
+ OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
+if (out == INVALID_HANDLE_VALUE) {
+error_setg(errp, "open %s failed", file->out);
+return NULL;
+}
+return qemu_chr_open_win_file(out);
+}
+
+#else /* WIN32 */
+
+static int qmp_chardev_open_file_source(char *src, int flags,
+Error **errp)
+{
+int fd = -1;
+
+TFR(fd = qemu_open(src, flags, 0666));
+if (fd == -1) {
+error_setg(errp, "open %s: %s", src, strerror(errno));
+}
+return fd;
+}
+
+static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
+{
+int flags, in = -1, out = -1;
+
+flags = O_WRONLY | O_TRUNC | O_CREAT | O_BINARY;
+out = qmp_chardev_open_file_source(file->out, flags, errp);
+if (error_is_set(errp)) {
+return NULL;
+}
+
+if (file->in) {
+flags = O_RDONLY;
+in = qmp_chardev_open_file_source(file->in, flags, errp);
+if (error_is_set(errp)) {
+qemu_close(out);
+return NULL;
+}
+}
+
+return qemu_chr_open_fd(in, out);
+}
+
+#endif /* WIN32 */
+
ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
Error **errp)
{
@@ -2952,6 +3010,9 @@ ChardevReturn *qmp_chardev_add(const char *id,
ChardevBackend *backend,
}
switch (backend->kind) {
+case CHARDEV_BACKEND_KIND_FILE:
+chr = qmp_chardev_open_file(backend->file, errp);
+break;
case CHARDEV_BACKEND_KIND_NULL:
chr = qemu_chr_open_null(NULL);
break;
diff --git a/qmp-commands.hx b/qmp-commands.hx
index c9ab37c..4d382f4 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2672,13 +2672,19 @@ Arguments:
- "id": the chardev's ID, must be unique (json-string)
- "backend": chardev backend type + parameters
-Example:
+Examples:
-> { "execute" : "chardev-add",
"arguments" : { "id" : "foo",
"backend" : { "type" : "null", "data" : {} } } }
<- { "return": {} }
+-> { "execute" : "chardev-add",
+ "arguments" : { "id" : "bar",
+ "backend" : { "type" : "file",
+ "data" : { "out" : "/tmp/bar.log" } } } }
+<- { "return": {} }
+
EQMP
{
--
1.7.1
[Qemu-devel] [PATCH v3 00/10] chardev hotplug patch series
Hi, Next chardev hotplug round. Minor API tweak (s/delay/nodelay/ for ChardevSocket). More documentation improvements. Added sanity checks. cheers, Gerd Gerd Hoffmann (10): chardev: add error reporting for qemu_chr_new_from_opts chardev: fix QemuOpts lifecycle chardev: reduce chardev ifdef mess a bit chardev: add qmp hotplug commands, with null chardev support chardev: add hmp hotplug commands chardev: add file chardev support to chardev-add (qmp) chardev: add serial chardev support to chardev-add (qmp) chardev: add parallel chardev support to chardev-add (qmp) chardev: add socket chardev support to chardev-add (qmp) chardev: add pty chardev support to chardev-add (qmp) hmp-commands.hx | 32 hmp.c | 23 +++ hmp.h |2 + include/char/char.h |4 +- qapi-schema.json| 104 qemu-char.c | 464 +++ qemu-options.hx | 14 +- qmp-commands.hx | 61 +++ vl.c|9 +- 9 files changed, 600 insertions(+), 113 deletions(-)
[Qemu-devel] [PATCH v3 09/10] chardev: add socket chardev support to chardev-add (qmp)
qemu_chr_open_socket is splitted into two functions. All initialization
after creating the socket file handler is splitted away into the new
qemu_chr_open_socket_fd function.
chr->filename doesn't get filled from QemuOpts any more. Qemu gathers
the information using getsockname and getnameinfo instead. This way it
will also work correctly for file handles passed via file descriptor
passing.
Finally qmp_chardev_open_socket() is the actual qmp hotplug
implementation which basically just calls socket_listen or
socket_connect and the new qemu_chr_open_socket_fd function.
Signed-off-by: Gerd Hoffmann
---
qapi-schema.json | 28 -
qemu-char.c | 168 -
2 files changed, 139 insertions(+), 57 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 320fa6b..5c3e3eb 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3049,6 +3049,27 @@
'type' : 'ChardevPortKind'} }
##
+# @ChardevSocket:
+#
+# Configuration info for socket chardevs.
+#
+# @addr: socket address to listen on (server=true)
+#or connect to (server=false)
+# @server: #optional create server socket (default: true)
+# @wait: #optional wait for connect (not used for server
+#sockets, default: false)
+# @nodelay: #optional set TCP_NODELAY socket option (default: false)
+# @telnet: #optional enable telnet protocol (default: false)
+#
+# Since: 1.4
+##
+{ 'type': 'ChardevSocket', 'data': { 'addr' : 'SocketAddress',
+ '*server' : 'bool',
+ '*wait': 'bool',
+ '*nodelay' : 'bool',
+ '*telnet' : 'bool' } }
+
+##
# @ChardevBackend:
#
# Configuration info for the new chardev backend.
@@ -3057,9 +3078,10 @@
##
{ 'type': 'ChardevDummy', 'data': { } }
-{ 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile',
- 'port' : 'ChardevPort',
- 'null' : 'ChardevDummy' } }
+{ 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile',
+ 'port' : 'ChardevPort',
+ 'socket' : 'ChardevSocket',
+ 'null' : 'ChardevDummy' } }
##
# @ChardevReturn:
diff --git a/qemu-char.c b/qemu-char.c
index 666ae18..5bd2138 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2438,10 +2438,88 @@ static void tcp_chr_close(CharDriverState *chr)
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
-static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
+static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
+bool is_listen, bool is_telnet,
+bool is_waitconnect,
+Error **errp)
{
CharDriverState *chr = NULL;
TCPCharDriver *s = NULL;
+char host[NI_MAXHOST], serv[NI_MAXSERV];
+const char *left = "", *right = "";
+struct sockaddr_storage ss;
+socklen_t ss_len = sizeof(ss);
+
+memset(&ss, 0, ss_len);
+if (getsockname(fd, (struct sockaddr *) &ss, &ss_len) != 0) {
+error_setg(errp, "getsockname: %s", strerror(errno));
+return NULL;
+}
+
+chr = g_malloc0(sizeof(CharDriverState));
+s = g_malloc0(sizeof(TCPCharDriver));
+
+s->connected = 0;
+s->fd = -1;
+s->listen_fd = -1;
+s->msgfd = -1;
+
+chr->filename = g_malloc(256);
+switch (ss.ss_family) {
+#ifndef _WIN32
+case AF_UNIX:
+s->is_unix = 1;
+snprintf(chr->filename, 256, "unix:%s%s",
+ ((struct sockaddr_un *)(&ss))->sun_path,
+ is_listen ? ",server" : "");
+break;
+#endif
+case AF_INET6:
+left = "[";
+right = "]";
+/* fall through */
+case AF_INET:
+s->do_nodelay = do_nodelay;
+getnameinfo((struct sockaddr *) &ss, ss_len, host, sizeof(host),
+serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
+snprintf(chr->filename, 256, "%s:%s:%s%s%s%s",
+ is_telnet ? "telnet" : "tcp",
+ left, host, right, serv,
+ is_listen ? ",server" : "");
+break;
+}
+
+chr->opaque = s;
+chr->chr_write = tcp_chr_write;
+chr->chr_close = tcp_chr_close;
+chr->get_msgfd = tcp_get_msgfd;
+chr->chr_add_client = tcp_chr_add_client;
+
+if (is_listen) {
+s->listen_fd = fd;
+qemu_set_fd_handler2(s->listen_fd, NULL, tcp_chr_accept, NULL, chr);
+if (is_telnet) {
+s->do_telnetopt = 1;
+}
+} else {
+s->connected = 1;
+s->fd = fd;
+socket_set_nodelay(fd);
+tcp_chr_connect(chr);
+}
+
+if (is_listen && is_waitconnect) {
+printf("QEMU waiting for
[Qemu-devel] [PATCH v3 03/10] chardev: reduce chardev ifdef mess a bit
Signed-off-by: Gerd Hoffmann
---
qemu-char.c | 22 +++---
1 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index a29c2bb..c511de3 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -856,6 +856,8 @@ static void cfmakeraw (struct termios *termios_p)
|| defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) \
|| defined(__GLIBC__)
+#define HAVE_CHARDEV_TTY 1
+
typedef struct {
int fd;
int connected;
@@ -1244,14 +1246,12 @@ static CharDriverState *qemu_chr_open_tty(QemuOpts
*opts)
chr->chr_close = qemu_chr_close_tty;
return chr;
}
-#else /* ! __linux__ && ! __sun__ */
-static CharDriverState *qemu_chr_open_pty(QemuOpts *opts)
-{
-return NULL;
-}
#endif /* __linux__ || __sun__ */
#if defined(__linux__)
+
+#define HAVE_CHARDEV_PARPORT 1
+
typedef struct {
int fd;
int mode;
@@ -1395,6 +1395,9 @@ static CharDriverState *qemu_chr_open_pp(QemuOpts *opts)
#endif /* __linux__ */
#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) ||
defined(__DragonFly__)
+
+#define HAVE_CHARDEV_PARPORT 1
+
static int pp_ioctl(CharDriverState *chr, int cmd, void *arg)
{
int fd = (int)(intptr_t)chr->opaque;
@@ -2755,19 +2758,16 @@ static const struct {
#else
{ .name = "file", .open = qemu_chr_open_file_out },
{ .name = "pipe", .open = qemu_chr_open_pipe },
-{ .name = "pty", .open = qemu_chr_open_pty },
{ .name = "stdio", .open = qemu_chr_open_stdio },
#endif
#ifdef CONFIG_BRLAPI
{ .name = "braille", .open = chr_baum_init },
#endif
-#if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \
-|| defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) \
-|| defined(__FreeBSD_kernel__)
+#ifdef HAVE_CHARDEV_TTY
{ .name = "tty", .open = qemu_chr_open_tty },
+{ .name = "pty", .open = qemu_chr_open_pty },
#endif
-#if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__) \
-|| defined(__FreeBSD_kernel__)
+#ifdef HAVE_CHARDEV_PARPORT
{ .name = "parport", .open = qemu_chr_open_pp },
#endif
#ifdef CONFIG_SPICE
--
1.7.1
[Qemu-devel] [PATCH v3 04/10] chardev: add qmp hotplug commands, with null chardev support
Add chardev-add and chardev-remove qmp commands. Hotplugging
a null chardev is supported for now, more will be added later.
Signed-off-by: Gerd Hoffmann
---
qapi-schema.json | 49 +
qemu-char.c | 52
qmp-commands.hx | 50 ++
3 files changed, 151 insertions(+), 0 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 5dfa052..462511e 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3017,3 +3017,52 @@
# Since: 1.3.0
##
{ 'command': 'nbd-server-stop' }
+
+##
+# @ChardevBackend:
+#
+# Configuration info for the new chardev backend.
+#
+# Since: 1.4
+##
+{ 'type': 'ChardevDummy', 'data': { } }
+
+{ 'union': 'ChardevBackend', 'data': { 'null' : 'ChardevDummy' } }
+
+##
+# @ChardevReturn:
+#
+# Return info about the chardev backend just created.
+#
+# Since: 1.4
+##
+{ 'type' : 'ChardevReturn', 'data': { } }
+
+##
+# @chardev-add:
+#
+# Add a file chardev
+#
+# @id: the chardev's ID, must be unique
+# @backend: backend type and parameters
+#
+# Returns: chardev info.
+#
+# Since: 1.4
+##
+{ 'command': 'chardev-add', 'data': {'id' : 'str',
+ 'backend' : 'ChardevBackend' },
+ 'returns': 'ChardevReturn' }
+
+##
+# @chardev-remove:
+#
+# Remove a chardev
+#
+# @id: the chardev's ID, must exist and not be in use
+#
+# Returns: Nothing on success
+#
+# Since: 1.4
+##
+{ 'command': 'chardev-remove', 'data': {'id': 'str'} }
diff --git a/qemu-char.c b/qemu-char.c
index c511de3..dd4acf5 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2938,3 +2938,55 @@ CharDriverState *qemu_char_get_next_serial(void)
return serial_hds[next_serial++];
}
+ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
+ Error **errp)
+{
+ChardevReturn *ret = g_new0(ChardevReturn, 1);
+CharDriverState *chr = NULL;
+
+chr = qemu_chr_find(id);
+if (chr) {
+error_setg(errp, "Chardev '%s' already exists", id);
+g_free(ret);
+return NULL;
+}
+
+switch (backend->kind) {
+case CHARDEV_BACKEND_KIND_NULL:
+chr = qemu_chr_open_null(NULL);
+break;
+default:
+error_setg(errp, "unknown chardev backend (%d)", backend->kind);
+break;
+}
+
+if (chr == NULL && !error_is_set(errp)) {
+error_setg(errp, "Failed to create chardev");
+}
+if (chr) {
+chr->label = g_strdup(id);
+chr->avail_connections = 1;
+QTAILQ_INSERT_TAIL(&chardevs, chr, next);
+return ret;
+} else {
+g_free(ret);
+return NULL;
+}
+}
+
+void qmp_chardev_remove(const char *id, Error **errp)
+{
+CharDriverState *chr;
+
+chr = qemu_chr_find(id);
+if (NULL == chr) {
+error_setg(errp, "Chardev '%s' not found", id);
+return;
+}
+if (chr->chr_can_read || chr->chr_read ||
+chr->chr_event || chr->handler_opaque) {
+error_setg(errp, "Chardev '%s' is busy", id);
+return;
+}
+qemu_chr_delete(chr);
+}
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 5c692d0..c9ab37c 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2654,3 +2654,53 @@ EQMP
.args_type = "",
.mhandler.cmd_new = qmp_marshal_input_query_target,
},
+
+{
+.name = "chardev-add",
+.args_type = "id:s,backend:q",
+.mhandler.cmd_new = qmp_marshal_input_chardev_add,
+},
+
+SQMP
+chardev-add
+
+
+Add a chardev.
+
+Arguments:
+
+- "id": the chardev's ID, must be unique (json-string)
+- "backend": chardev backend type + parameters
+
+Example:
+
+-> { "execute" : "chardev-add",
+ "arguments" : { "id" : "foo",
+ "backend" : { "type" : "null", "data" : {} } } }
+<- { "return": {} }
+
+EQMP
+
+{
+.name = "chardev-remove",
+.args_type = "id:s",
+.mhandler.cmd_new = qmp_marshal_input_chardev_remove,
+},
+
+
+SQMP
+chardev-remove
+--
+
+Remove a chardev.
+
+Arguments:
+
+- "id": the chardev's ID, must exist and not be in use (json-string)
+
+Example:
+
+-> { "execute": "chardev-remove", "arguments": { "id" : "foo" } }
+<- { "return": {} }
+
+EQMP
--
1.7.1
Re: [Qemu-devel] [PATCH 6/6] hw/pc.c: Fix converting of ioport_register* to MemoryRegion
Am 11.01.2013 10:18, schrieb Stefan Hajnoczi:
> From: Julien Grall
>
> The commit 258711 introduced MemoryRegion to replace ioport_region*
> for ioport 80h and F0h.
> A MemoryRegion needs to have both read and write callback otherwise a segfault
> will occur when an access is made.
>
> The previous behaviour of this both ioport is to return 0x.
> So keep this behaviour.
>
> Reported-by: Adam Lackorzynski
> Signed-off-by: Julien Grall
> Tested-by: Adam Lackorzynski
> Signed-off-by: Stefan Hajnoczi
> ---
> hw/pc.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index df0c48e..90b1bf7 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -103,6 +103,11 @@ static void ioport80_write(void *opaque, hwaddr addr,
> uint64_t data,
> {
> }
>
> +static uint64_t ioport80_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +return 0x;
Might these require ULL for i386?
Andreas
> +}
> +
> /* MSDOS compatibility mode FPU exception support */
> static qemu_irq ferr_irq;
>
> @@ -123,6 +128,11 @@ static void ioportF0_write(void *opaque, hwaddr addr,
> uint64_t data,
> qemu_irq_lower(ferr_irq);
> }
>
> +static uint64_t ioportF0_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +return 0x;
> +}
> +
> /* TSC handling */
> uint64_t cpu_get_tsc(CPUX86State *env)
> {
> @@ -960,6 +970,7 @@ static void cpu_request_exit(void *opaque, int irq, int
> level)
>
> static const MemoryRegionOps ioport80_io_ops = {
> .write = ioport80_write,
> +.read = ioport80_read,
> .endianness = DEVICE_NATIVE_ENDIAN,
> .impl = {
> .min_access_size = 1,
> @@ -969,6 +980,7 @@ static const MemoryRegionOps ioport80_io_ops = {
>
> static const MemoryRegionOps ioportF0_io_ops = {
> .write = ioportF0_write,
> +.read = ioportF0_read,
> .endianness = DEVICE_NATIVE_ENDIAN,
> .impl = {
> .min_access_size = 1,
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [PATCH v3 02/10] chardev: fix QemuOpts lifecycle
qemu_chr_new_from_opts handles QemuOpts release now, so callers don't
have to worry. It will either be saved in CharDriverState, then
released in qemu_chr_delete, or in the error case released instantly.
Signed-off-by: Gerd Hoffmann
---
include/char/char.h |1 +
qemu-char.c | 20 ++--
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/include/char/char.h b/include/char/char.h
index 1952a10..c91ce3c 100644
--- a/include/char/char.h
+++ b/include/char/char.h
@@ -75,6 +75,7 @@ struct CharDriverState {
char *filename;
int opened;
int avail_connections;
+QemuOpts *opts;
QTAILQ_ENTRY(CharDriverState) next;
};
diff --git a/qemu-char.c b/qemu-char.c
index 91f64e9..a29c2bb 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2787,13 +2787,13 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
if (qemu_opts_id(opts) == NULL) {
error_setg(errp, "chardev: no id specified\n");
-return NULL;
+goto err;
}
if (qemu_opt_get(opts, "backend") == NULL) {
error_setg(errp, "chardev: \"%s\" missing backend\n",
qemu_opts_id(opts));
-return NULL;
+goto err;
}
for (i = 0; i < ARRAY_SIZE(backend_table); i++) {
if (strcmp(backend_table[i].name, qemu_opt_get(opts, "backend")) == 0)
@@ -2802,14 +2802,14 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
if (i == ARRAY_SIZE(backend_table)) {
error_setg(errp, "chardev: backend \"%s\" not found\n",
qemu_opt_get(opts, "backend"));
-return NULL;
+goto err;
}
chr = backend_table[i].open(opts);
if (!chr) {
error_setg(errp, "chardev: opening backend \"%s\" failed\n",
qemu_opt_get(opts, "backend"));
-return NULL;
+goto err;
}
if (!chr->filename)
@@ -2830,7 +2830,12 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
chr->avail_connections = 1;
}
chr->label = g_strdup(qemu_opts_id(opts));
+chr->opts = opts;
return chr;
+
+err:
+qemu_opts_del(opts);
+return NULL;
}
CharDriverState *qemu_chr_new(const char *label, const char *filename, void
(*init)(struct CharDriverState *s))
@@ -2856,7 +2861,6 @@ CharDriverState *qemu_chr_new(const char *label, const
char *filename, void (*in
if (chr && qemu_opt_get_bool(opts, "mux", 0)) {
monitor_init(chr, MONITOR_USE_READLINE);
}
-qemu_opts_del(opts);
return chr;
}
@@ -2884,10 +2888,14 @@ void qemu_chr_fe_close(struct CharDriverState *chr)
void qemu_chr_delete(CharDriverState *chr)
{
QTAILQ_REMOVE(&chardevs, chr, next);
-if (chr->chr_close)
+if (chr->chr_close) {
chr->chr_close(chr);
+}
g_free(chr->filename);
g_free(chr->label);
+if (chr->opts) {
+qemu_opts_del(chr->opts);
+}
g_free(chr);
}
--
1.7.1
[Qemu-devel] [PATCH v3 07/10] chardev: add serial chardev support to chardev-add (qmp)
Similar to file, except that no separate in/out files are supported
because it's pointless for direct device access. Also the special
tty ioctl hooks (pass through linespeed settings etc) are activated
on Unix.
Signed-off-by: Gerd Hoffmann
---
qapi-schema.json | 17 ++
qemu-char.c | 62 +++---
qemu-options.hx |9 +++
3 files changed, 75 insertions(+), 13 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index c70c118..d33 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3032,6 +3032,22 @@
'out' : 'str' } }
##
+# @ChardevPort:
+#
+# Configuration info for device chardevs.
+#
+# @device: The name of the special file for the device,
+# i.e. /dev/ttyS0 on Unix or COM1: on Windows
+# @type: What kind of device this is.
+#
+# Since: 1.4
+##
+{ 'enum': 'ChardevPortKind', 'data': [ 'serial' ] }
+
+{ 'type': 'ChardevPort', 'data': { 'device' : 'str',
+ 'type' : 'ChardevPortKind'} }
+
+##
# @ChardevBackend:
#
# Configuration info for the new chardev backend.
@@ -3041,6 +3057,7 @@
{ 'type': 'ChardevDummy', 'data': { } }
{ 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile',
+ 'port' : 'ChardevPort',
'null' : 'ChardevDummy' } }
##
diff --git a/qemu-char.c b/qemu-char.c
index b8b5f16..b72f968 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1230,21 +1230,27 @@ static void qemu_chr_close_tty(CharDriverState *chr)
}
}
+static CharDriverState *qemu_chr_open_tty_fd(int fd)
+{
+CharDriverState *chr;
+
+tty_serial_init(fd, 115200, 'N', 8, 1);
+chr = qemu_chr_open_fd(fd, fd);
+chr->chr_ioctl = tty_serial_ioctl;
+chr->chr_close = qemu_chr_close_tty;
+return chr;
+}
+
static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
{
const char *filename = qemu_opt_get(opts, "path");
-CharDriverState *chr;
int fd;
TFR(fd = qemu_open(filename, O_RDWR | O_NONBLOCK));
if (fd < 0) {
return NULL;
}
-tty_serial_init(fd, 115200, 'N', 8, 1);
-chr = qemu_chr_open_fd(fd, fd);
-chr->chr_ioctl = tty_serial_ioctl;
-chr->chr_close = qemu_chr_close_tty;
-return chr;
+return qemu_chr_open_tty_fd(fd);
}
#endif /* __linux__ || __sun__ */
@@ -1666,9 +1672,8 @@ static int win_chr_poll(void *opaque)
return 0;
}
-static CharDriverState *qemu_chr_open_win(QemuOpts *opts)
+static CharDriverState *qemu_chr_open_win_path(const char *filename)
{
-const char *filename = qemu_opt_get(opts, "path");
CharDriverState *chr;
WinCharState *s;
@@ -1687,6 +1692,11 @@ static CharDriverState *qemu_chr_open_win(QemuOpts *opts)
return chr;
}
+static CharDriverState *qemu_chr_open_win(QemuOpts *opts)
+{
+return qemu_chr_open_win_path(qemu_opt_get(opts, "path"));
+}
+
static int win_chr_pipe_poll(void *opaque)
{
CharDriverState *chr = opaque;
@@ -2765,6 +2775,7 @@ static const struct {
#endif
#ifdef HAVE_CHARDEV_TTY
{ .name = "tty", .open = qemu_chr_open_tty },
+{ .name = "serial",.open = qemu_chr_open_tty },
{ .name = "pty", .open = qemu_chr_open_pty },
#endif
#ifdef HAVE_CHARDEV_PARPORT
@@ -2958,6 +2969,17 @@ static CharDriverState
*qmp_chardev_open_file(ChardevFile *file, Error **errp)
return qemu_chr_open_win_file(out);
}
+static CharDriverState *qmp_chardev_open_port(ChardevPort *port, Error **errp)
+{
+switch (port->type) {
+case CHARDEV_PORT_KIND_SERIAL:
+return qemu_chr_open_win_path(port->device);
+default:
+error_setg(errp, "unknown chardev port (%d)", port->type);
+return NULL;
+}
+}
+
#else /* WIN32 */
static int qmp_chardev_open_file_source(char *src, int flags,
@@ -2994,6 +3016,27 @@ static CharDriverState
*qmp_chardev_open_file(ChardevFile *file, Error **errp)
return qemu_chr_open_fd(in, out);
}
+static CharDriverState *qmp_chardev_open_port(ChardevPort *port, Error **errp)
+{
+int flags, fd;
+
+switch (port->type) {
+#ifdef HAVE_CHARDEV_TTY
+case CHARDEV_PORT_KIND_SERIAL:
+flags = O_RDWR;
+fd = qmp_chardev_open_file_source(port->device, flags, errp);
+if (error_is_set(errp)) {
+return NULL;
+}
+socket_set_nonblock(fd);
+return qemu_chr_open_tty_fd(fd);
+#endif
+default:
+error_setg(errp, "unknown chardev port (%d)", port->type);
+return NULL;
+}
+}
+
#endif /* WIN32 */
ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
@@ -3013,6 +3056,9 @@ ChardevReturn *qmp_chardev_add(const char *id,
ChardevBackend *backend,
case CHARDEV_BACKEND_KIND_FILE:
chr = qmp_chardev_open_file(backend->file, errp);
break;
+case CHARDEV_BACKEND_KIND_PORT:
+chr = qmp_chardev_open_port(backend->port, errp);
+
[Qemu-devel] [PATCH v3 08/10] chardev: add parallel chardev support to chardev-add (qmp)
Also alias the old parport name to parallel for -chardev.
Signed-off-by: Gerd Hoffmann
---
qapi-schema.json |3 ++-
qemu-char.c | 44
qemu-options.hx |5 -
3 files changed, 34 insertions(+), 18 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index d33..320fa6b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3042,7 +3042,8 @@
#
# Since: 1.4
##
-{ 'enum': 'ChardevPortKind', 'data': [ 'serial' ] }
+{ 'enum': 'ChardevPortKind', 'data': [ 'serial',
+ 'parallel' ] }
{ 'type': 'ChardevPort', 'data': { 'device' : 'str',
'type' : 'ChardevPortKind'} }
diff --git a/qemu-char.c b/qemu-char.c
index b72f968..666ae18 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1367,17 +1367,10 @@ static void pp_close(CharDriverState *chr)
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
-static CharDriverState *qemu_chr_open_pp(QemuOpts *opts)
+static CharDriverState *qemu_chr_open_pp_fd(int fd)
{
-const char *filename = qemu_opt_get(opts, "path");
CharDriverState *chr;
ParallelCharDriver *drv;
-int fd;
-
-TFR(fd = qemu_open(filename, O_RDWR));
-if (fd < 0) {
-return NULL;
-}
if (ioctl(fd, PPCLAIM) < 0) {
close(fd);
@@ -1441,16 +1434,9 @@ static int pp_ioctl(CharDriverState *chr, int cmd, void
*arg)
return 0;
}
-static CharDriverState *qemu_chr_open_pp(QemuOpts *opts)
+static CharDriverState *qemu_chr_open_pp_fd(int fd)
{
-const char *filename = qemu_opt_get(opts, "path");
CharDriverState *chr;
-int fd;
-
-fd = qemu_open(filename, O_RDWR);
-if (fd < 0) {
-return NULL;
-}
chr = g_malloc0(sizeof(CharDriverState));
chr->opaque = (void *)(intptr_t)fd;
@@ -2750,6 +2736,22 @@ fail:
return NULL;
}
+#ifdef HAVE_CHARDEV_PARPORT
+
+static CharDriverState *qemu_chr_open_pp(QemuOpts *opts)
+{
+const char *filename = qemu_opt_get(opts, "path");
+int fd;
+
+fd = qemu_open(filename, O_RDWR);
+if (fd < 0) {
+return NULL;
+}
+return qemu_chr_open_pp_fd(fd);
+}
+
+#endif
+
static const struct {
const char *name;
CharDriverState *(*open)(QemuOpts *opts);
@@ -2779,6 +2781,7 @@ static const struct {
{ .name = "pty", .open = qemu_chr_open_pty },
#endif
#ifdef HAVE_CHARDEV_PARPORT
+{ .name = "parallel", .open = qemu_chr_open_pp },
{ .name = "parport", .open = qemu_chr_open_pp },
#endif
#ifdef CONFIG_SPICE
@@ -3031,6 +3034,15 @@ static CharDriverState
*qmp_chardev_open_port(ChardevPort *port, Error **errp)
socket_set_nonblock(fd);
return qemu_chr_open_tty_fd(fd);
#endif
+#ifdef HAVE_CHARDEV_PARPORT
+case CHARDEV_PORT_KIND_PARALLEL:
+flags = O_RDWR;
+fd = qmp_chardev_open_file_source(port->device, flags, errp);
+if (error_is_set(errp)) {
+return NULL;
+}
+return qemu_chr_open_pp_fd(fd);
+#endif
default:
error_setg(errp, "unknown chardev port (%d)", port->type);
return NULL;
diff --git a/qemu-options.hx b/qemu-options.hx
index 17cc1ad..40cd683 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1746,6 +1746,7 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
"-chardev tty,id=id,path=path[,mux=on|off]\n"
#endif
#if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__)
+"-chardev parallel,id=id,path=path[,mux=on|off]\n"
"-chardev parport,id=id,path=path[,mux=on|off]\n"
#endif
#if defined(CONFIG_SPICE)
@@ -1776,6 +1777,7 @@ Backend is one of:
@option{stdio},
@option{braille},
@option{tty},
+@option{parallel},
@option{parport},
@option{spicevmc}.
@option{spiceport}.
@@ -1943,9 +1945,10 @@ DragonFlyBSD hosts. It is an alias for -serial.
@option{path} specifies the path to the tty. @option{path} is required.
+@item -chardev parallel ,id=@var{id} ,path=@var{path}
@item -chardev parport ,id=@var{id} ,path=@var{path}
-@option{parport} is only available on Linux, FreeBSD and DragonFlyBSD hosts.
+@option{parallel} is only available on Linux, FreeBSD and DragonFlyBSD hosts.
Connect to a local parallel port.
--
1.7.1
[Qemu-devel] [PATCH v3 10/10] chardev: add pty chardev support to chardev-add (qmp)
The ptsname is returned directly, so there is no need to
use query-chardev to figure the pty device path.
Signed-off-by: Gerd Hoffmann
---
qapi-schema.json |3 ++-
qemu-char.c | 13 +
qmp-commands.hx |5 +
3 files changed, 20 insertions(+), 1 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 5c3e3eb..6d7252b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3081,6 +3081,7 @@
{ 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile',
'port' : 'ChardevPort',
'socket' : 'ChardevSocket',
+ 'pty': 'ChardevDummy',
'null' : 'ChardevDummy' } }
##
@@ -3090,7 +3091,7 @@
#
# Since: 1.4
##
-{ 'type' : 'ChardevReturn', 'data': { } }
+{ 'type' : 'ChardevReturn', 'data': { '*pty' : 'str' } }
##
# @chardev-add:
diff --git a/qemu-char.c b/qemu-char.c
index 5bd2138..a1bed70 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3131,6 +3131,19 @@ ChardevReturn *qmp_chardev_add(const char *id,
ChardevBackend *backend,
case CHARDEV_BACKEND_KIND_SOCKET:
chr = qmp_chardev_open_socket(backend->socket, errp);
break;
+#ifdef HAVE_CHARDEV_TTY
+case CHARDEV_BACKEND_KIND_PTY:
+{
+/* qemu_chr_open_pty sets "path" in opts */
+QemuOpts *opts;
+opts = qemu_opts_create_nofail(qemu_find_opts("chardev"));
+chr = qemu_chr_open_pty(opts);
+ret->pty = g_strdup(qemu_opt_get(opts, "path"));
+ret->has_pty = true;
+qemu_opts_del(opts);
+break;
+}
+#endif
case CHARDEV_BACKEND_KIND_NULL:
chr = qemu_chr_open_null(NULL);
break;
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 4d382f4..cbf1280 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2685,6 +2685,11 @@ Examples:
"data" : { "out" : "/tmp/bar.log" } } } }
<- { "return": {} }
+-> { "execute" : "chardev-add",
+ "arguments" : { "id" : "baz",
+ "backend" : { "type" : "pty", "data" : {} } } }
+<- { "return": { "pty" : "/dev/pty/42" } }
+
EQMP
{
--
1.7.1
Re: [Qemu-devel] [RFC PATCH] PIIX: reset the VM when the Reset Control Register's RCPU bit gets set
Hi,
Am 11.01.2013 10:30, schrieb Laszlo Ersek:
> On 01/09/13 22:01, Blue Swirl wrote:
>> On Tue, Jan 8, 2013 at 9:44 PM, Laszlo Ersek wrote:
>
>>> +static void i440fx_host_config_write(void *opaque, hwaddr addr,
>>> + uint64_t val, unsigned len)
>>> +{
>>> +if (addr == 1 && len == 1) {
>>> +if (val & 4) {
>>> +qemu_system_reset_request();
>>> +}
>>> +return;
>>> +}
>>> +pci_host_conf_le_ops.write(opaque, addr, val, len);
>>> +}
>>> +
>>> +static MemoryRegionOps i440fx_host_conf_ops = {
>>> +.read = NULL,
>>> +.write = i440fx_host_config_write,
>>> +.endianness = DEVICE_LITTLE_ENDIAN
>>> +};
>>> +
>>> static int i440fx_pcihost_initfn(SysBusDevice *dev)
>>> {
>>> PCIHostState *s = PCI_HOST_BRIDGE(dev);
>>>
>>> -memory_region_init_io(&s->conf_mem, &pci_host_conf_le_ops, s,
>>> +i440fx_host_conf_ops.read = pci_host_conf_le_ops.read;
>>
>> It would be cleaner to introduce a new memory region (without this
>> copying) which passes 0xcf8 and 0xcfc to standard PCI host but catches
>> accesses to 0xcf9. This may mean that pci_host_config_{read,write}
>> will need to be exposed.
>
> Do you mean:
>
> (1) introducing the new "i440fx_host_conf_ops" struct-of-funcptrs with
> detached functions (that is, duplicating the guts of
> pci_host_config_{read,write} and modifying them, and then registering
> s->conf_mem with this "i440fx_host_conf_ops"; or
>
> (2) leaving s->conf_mem as-is, and introducing a sub-region just for
> port 0xcf9, with higher visibility priority?
>
> (I don't feel confident about (2), and based on "docs/memory.txt" I
> thought that overlapping regions had not been invented for this purpose.)
>
> IOW, are you OK with the explicit offset + access-width based check,
> just organized differently, or are you proposing a one-byte-wide subregion?
Another option:
(3) leaving s->conf_mem as-is but implementing your own read function as
well that forwards to pci_host_conf_le_ops.read() to avoid this unusual
non-const MemoryRegionOps construct
But I guess Blue meant (2), which should be slightly more performant.
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [PATCH v3 01/10] chardev: add error reporting for qemu_chr_new_from_opts
Signed-off-by: Gerd Hoffmann
---
include/char/char.h |3 ++-
qemu-char.c | 24 +++-
vl.c|9 ++---
3 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/include/char/char.h b/include/char/char.h
index baa5d03..1952a10 100644
--- a/include/char/char.h
+++ b/include/char/char.h
@@ -89,7 +89,8 @@ struct CharDriverState {
* Returns: a new character backend
*/
CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
-void (*init)(struct CharDriverState *s));
+void (*init)(struct CharDriverState *s),
+Error **errp);
/**
* @qemu_chr_new:
diff --git a/qemu-char.c b/qemu-char.c
index f41788c..91f64e9 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2779,19 +2779,20 @@ static const struct {
};
CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
-void (*init)(struct CharDriverState *s))
+void (*init)(struct CharDriverState *s),
+Error **errp)
{
CharDriverState *chr;
int i;
if (qemu_opts_id(opts) == NULL) {
-fprintf(stderr, "chardev: no id specified\n");
+error_setg(errp, "chardev: no id specified\n");
return NULL;
}
if (qemu_opt_get(opts, "backend") == NULL) {
-fprintf(stderr, "chardev: \"%s\" missing backend\n",
-qemu_opts_id(opts));
+error_setg(errp, "chardev: \"%s\" missing backend\n",
+ qemu_opts_id(opts));
return NULL;
}
for (i = 0; i < ARRAY_SIZE(backend_table); i++) {
@@ -2799,15 +2800,15 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
break;
}
if (i == ARRAY_SIZE(backend_table)) {
-fprintf(stderr, "chardev: backend \"%s\" not found\n",
-qemu_opt_get(opts, "backend"));
+error_setg(errp, "chardev: backend \"%s\" not found\n",
+ qemu_opt_get(opts, "backend"));
return NULL;
}
chr = backend_table[i].open(opts);
if (!chr) {
-fprintf(stderr, "chardev: opening backend \"%s\" failed\n",
-qemu_opt_get(opts, "backend"));
+error_setg(errp, "chardev: opening backend \"%s\" failed\n",
+ qemu_opt_get(opts, "backend"));
return NULL;
}
@@ -2837,6 +2838,7 @@ CharDriverState *qemu_chr_new(const char *label, const
char *filename, void (*in
const char *p;
CharDriverState *chr;
QemuOpts *opts;
+Error *err = NULL;
if (strstart(filename, "chardev:", &p)) {
return qemu_chr_find(p);
@@ -2846,7 +2848,11 @@ CharDriverState *qemu_chr_new(const char *label, const
char *filename, void (*in
if (!opts)
return NULL;
-chr = qemu_chr_new_from_opts(opts, init);
+chr = qemu_chr_new_from_opts(opts, init, &err);
+if (error_is_set(&err)) {
+fprintf(stderr, "%s\n", error_get_pretty(err));
+error_free(err);
+}
if (chr && qemu_opt_get_bool(opts, "mux", 0)) {
monitor_init(chr, MONITOR_USE_READLINE);
}
diff --git a/vl.c b/vl.c
index 79e5122..b4ea3ae 100644
--- a/vl.c
+++ b/vl.c
@@ -2052,11 +2052,14 @@ static int device_init_func(QemuOpts *opts, void
*opaque)
static int chardev_init_func(QemuOpts *opts, void *opaque)
{
-CharDriverState *chr;
+Error *local_err = NULL;
-chr = qemu_chr_new_from_opts(opts, NULL);
-if (!chr)
+qemu_chr_new_from_opts(opts, NULL, &local_err);
+if (error_is_set(&local_err)) {
+fprintf(stderr, "%s\n", error_get_pretty(local_err));
+error_free(local_err);
return -1;
+}
return 0;
}
--
1.7.1
Re: [Qemu-devel] [RFC PATCH] PIIX: reset the VM when the Reset Control Register's RCPU bit gets set
On 01/09/13 22:01, Blue Swirl wrote:
> On Tue, Jan 8, 2013 at 9:44 PM, Laszlo Ersek wrote:
>> static int i440fx_pcihost_initfn(SysBusDevice *dev)
>> {
>> PCIHostState *s = PCI_HOST_BRIDGE(dev);
>>
>> -memory_region_init_io(&s->conf_mem, &pci_host_conf_le_ops, s,
>> +i440fx_host_conf_ops.read = pci_host_conf_le_ops.read;
>
> It would be cleaner to introduce a new memory region (without this
> copying) which passes 0xcf8 and 0xcfc to standard PCI host but catches
> accesses to 0xcf9. This may mean that pci_host_config_{read,write}
> will need to be exposed.
(3) Do you have a single region in mind that covers [ 0xcf8 ..0xcff ]
contiguously? That would require exposing pci_host_data_{read,write}
too, not only pci_host_config_{read,write}.
Plus, the config & data regions of I440FXState.parent_obj have distinct
names now ("pci-conf-idx" and "pci-conf-data"); merging them (and
inventing a new name) might be user-visible via the "info mtree" monitor
command. (At least it would differ from many of the PCI host
implementations in the tree.)
What if I change v1 like this:
*
diff --git a/hw/pci/pci_host.h b/hw/pci/pci_host.h
index 1845d4d..f5b6487 100644
--- a/hw/pci/pci_host.h
+++ b/hw/pci/pci_host.h
@@ -53,6 +53,9 @@ uint32_t pci_host_config_read_common(PCIDevice *pci_dev,
uint32_t addr,
void pci_data_write(PCIBus *s, uint32_t addr, uint32_t val, int len);
uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len);
+void pci_host_config_write(void *opaque, hwaddr addr, uint64_t val,
+ unsigned len);
+uint64_t pci_host_config_read(void *opaque, hwaddr addr, unsigned len);
extern const MemoryRegionOps pci_host_conf_le_ops;
extern const MemoryRegionOps pci_host_conf_be_ops;
diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
index 265c324..b0f359d 100644
--- a/hw/pci/pci_host.c
+++ b/hw/pci/pci_host.c
@@ -94,8 +94,8 @@ uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
return val;
}
-static void pci_host_config_write(void *opaque, hwaddr addr,
- uint64_t val, unsigned len)
+void pci_host_config_write(void *opaque, hwaddr addr, uint64_t val,
+ unsigned len)
{
PCIHostState *s = opaque;
@@ -107,8 +107,7 @@ static void pci_host_config_write(void *opaque, hwaddr addr,
s->config_reg = val;
}
-static uint64_t pci_host_config_read(void *opaque, hwaddr addr,
- unsigned len)
+uint64_t pci_host_config_read(void *opaque, hwaddr addr, unsigned len)
{
PCIHostState *s = opaque;
uint32_t val = s->config_reg;
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 89d694c..168e20d 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -190,11 +190,11 @@ static void i440fx_host_config_write(void *opaque, hwaddr
addr,
}
return;
}
-pci_host_conf_le_ops.write(opaque, addr, val, len);
+pci_host_config_write(opaque, addr, val, len);
}
-static MemoryRegionOps i440fx_host_conf_ops = {
-.read = NULL,
+static const MemoryRegionOps i440fx_host_conf_ops = {
+.read = pci_host_conf_read,
.write = i440fx_host_config_write,
.endianness = DEVICE_LITTLE_ENDIAN
};
@@ -203,7 +203,6 @@ static int i440fx_pcihost_initfn(SysBusDevice *dev)
{
PCIHostState *s = PCI_HOST_BRIDGE(dev);
-i440fx_host_conf_ops.read = pci_host_conf_le_ops.read;
memory_region_init_io(&s->conf_mem, &i440fx_host_conf_ops, s,
"pci-conf-idx", 4);
sysbus_add_io(dev, 0xcf8, &s->conf_mem);
*
I'm not sure at which depth you want me to stop duplicating the "base class":
- call its functions via pci_host_conf_le_ops.FIELD (v1 rfc),
- call its functions by their direct names (exposing them first, the
above),
- instead of reusing "pci_host_data_le_ops" for "pci-conf-data", create
an "i440fx_host_DATA_ops" global as well:
- pointing at pci_host_data_{read,write} (exposing them first),
- pointing at private functions calling pci_host_data_{read,write}...
Thanks,
Laszlo
Re: [Qemu-devel] [PATCH scsi-ish] raw-posix: fix bdrv_aio_ioctl
Am 10.01.2013 15:28, schrieb Paolo Bonzini: > When the raw-posix aio=thread code was moved from posix-aio-compat.c > to block/raw-posix.c, there was an unintended change to the ioctl code. > The code used to return the ioctl command, which posix_aio_read() > would later morph into a zero. This hack is not necessary anymore, > and in fact breaks scsi-generic (which expects a zero return code). > Remove it. > > Cc: qemu-sta...@nongnu.org > Signed-off-by: Paolo Bonzini Thanks, applied to the block branch. Kevin
[Qemu-devel] [PATCH] block: fix initialization in bdrv_io_limits_enable()
bdrv_io_limits_enable() starts a new slice, but does not set io_base
correctly for that slice.
Here is how io_base is used:
bytes_base = bs->nr_bytes[is_write] - bs->io_base.bytes[is_write];
bytes_res = (unsigned) nb_sectors * BDRV_SECTOR_SIZE;
if (bytes_base + bytes_res <= bytes_limit) {
/* no wait */
} else {
/* operation needs to be throttled */
}
As a result, any I/O operations that are triggered between now and
bs->slice_end are incorrectly limited. If 10 MB of data has been
written since the VM was started, QEMU thinks that 10 MB of data has
been written in this slice. This leads to a I/O lockup in the guest.
We fix this by delaying the start of a new slice to the next
call of bdrv_exceed_io_limits().
Signed-off-by: Peter Lieven
---
block.c |4
1 file changed, 4 deletions(-)
diff --git a/block.c b/block.c
index 4e28c55..a40a389 100644
--- a/block.c
+++ b/block.c
@@ -155,10 +155,6 @@ void bdrv_io_limits_enable(BlockDriverState *bs)
{
qemu_co_queue_init(&bs->throttled_reqs);
bs->block_timer = qemu_new_timer_ns(vm_clock, bdrv_block_timer, bs);
-bs->slice_time = 5 * BLOCK_IO_SLICE_TIME;
-bs->slice_start = qemu_get_clock_ns(vm_clock);
-bs->slice_end = bs->slice_start + bs->slice_time;
-memset(&bs->io_base, 0, sizeof(bs->io_base));
bs->io_limits_enabled = true;
}
--
1.7.9.5
Re: [Qemu-devel] [PATCH 06/17] target-i386: remove vendor_override field from CPUX86State
On Fri, Jan 11, 2013 at 03:10:20AM +0100, Igor Mammedov wrote: > commit 8935499831312 makes cpuid return to guest host's vendor value > instead of built-in one by default if kvm_enabled() == true and allows > to override this behavior if 'vendor' is specified on -cpu command line. > > But every time guest calls cpuid to get 'vendor' value, host's value is > read again and again in default case. > > It complicates semantic of vendor property and makes it harder to use. > > Instead of reading 'vendor' value from host every time cpuid[vendor] is > called, override 'vendor' value only once in cpu_x86_find_by_name(), when > built-in CPU model is found and if(kvm_enabled() == true). > > It provides the same default semantic > if (kvm_enabled() == true) vendor = host's vendor > else vendor = built-in vendor > > and then later: > if (custom vendor) vendor = custom vendor > > 'vendor' value is overridden when user provides it on -cpu command line, > and there isn't need in vendor_override field anymore, remove it. > > Signed-off-by: Igor Mammedov In case my previous reviewed-by line (buried in the discussion about property defaults) was missed: Reviewed-by: Eduardo Habkost > --- > v4: >- rebased with "target-i386: move out CPU features initialization > in separate func" dropped. So remove vendor_override in > cpu_x86_register() instead. >- replace x86cpu_vendor_words2str() with x86_cpu_vendor_words2str() > due to renaming of the last in previous patch > --- > target-i386/cpu.c | 27 --- > target-i386/cpu.h |1 - > 2 files changed, 12 insertions(+), 16 deletions(-) > [...] -- Eduardo
Re: [Qemu-devel] [PATCH 06/17] target-i386: remove vendor_override field from CPUX86State
On Fri, Jan 11, 2013 at 04:09:45AM +0100, Igor Mammedov wrote:
> On Fri, 11 Jan 2013 00:42:13 -0200
> Eduardo Habkost wrote:
>
> > On Fri, Jan 11, 2013 at 03:10:20AM +0100, Igor Mammedov wrote:
> > > commit 8935499831312 makes cpuid return to guest host's vendor value
> > > instead of built-in one by default if kvm_enabled() == true and allows
> > > to override this behavior if 'vendor' is specified on -cpu command line.
> > >
> > > But every time guest calls cpuid to get 'vendor' value, host's value is
> > > read again and again in default case.
> > >
> > > It complicates semantic of vendor property and makes it harder to use.
> > >
> > > Instead of reading 'vendor' value from host every time cpuid[vendor] is
> > > called, override 'vendor' value only once in cpu_x86_find_by_name(), when
> > > built-in CPU model is found and if(kvm_enabled() == true).
> > >
> > > It provides the same default semantic
> > > if (kvm_enabled() == true) vendor = host's vendor
> > > else vendor = built-in vendor
> > >
> > > and then later:
> > > if (custom vendor) vendor = custom vendor
> > >
> >
> > How do you plan to make this work when the vendor string for each model
> > gets represented using a default defined at class_init-time[1]? When we do
> it could be done by late update hook like you did for host cpu subclass or
> forcing early kvm_init() and then do it at class_init-time. I still hope that
> second option would work(but I haven't checked yet).
I believe that it is by design that class_init runs before kvm={on,off}
configuration is handled, and we won't be able to change it. We'll need
to clarify that.
And I don't believe we will be allowed to change the class defaults
after class_init already ran. That may be a problem for -cpu "host" as
well. Maybe in the case of -cpu "host" we will need to translate the
behavior to a "enable-all-features-supported-by-the-host=true" property
that would be handled by instance_init and override all f-* flags, or
making the feature flags tristate (on/off/host), and setting
"f-feature=host" for all features.
>
> > that, we wouldn't be able to differentiate the built/predefined vendor
> > value (used for TCG only) and the user-defined vendor value (that would
> > be set in KVM mode too).
> If we have subclasses with already correct vendor for a specific mode then we
> needn't to know if it is built-in vendor or user-defined.
True, but I still don't see an obvious way to make sure the class has
the already correct vendor for a specific mode.
That's why having separate "vendor"/"tcg-vendor" (and maybe even
"kvm-vendor") properties sounded like a feasible solution.
Anyway, this shouldn't hold this specific patch, and we will be forced
to discuss that when we have a patch that adds a "vendor" static
property, so:
Reviewed-by: Eduardo Habkost
>
> >
> > [1] AFAIU, class_init would never be able to check kvm_enabled(), by
> > design, because it may run before anything is configured (even the
> > choice to enable/disable KVM).
> >
> > Maybe we can later add a "tcg-vendor" property, that would override the
> > vendor only if in TCG mode. Then the predefined CPU models could have a
> > "tcg-vendor" default set, and no "vendor" default set, to emulate the
> > current behavior.
> >
> > > 'vendor' value is overridden when user provides it on -cpu command line,
> > > and there isn't need in vendor_override field anymore, remove it.
> > >
> > > Signed-off-by: Igor Mammedov
> > > ---
> > > v4:
> > >- rebased with "target-i386: move out CPU features initialization
> > > in separate func" dropped. So remove vendor_override in
> > > cpu_x86_register() instead.
> > >- replace x86cpu_vendor_words2str() with x86_cpu_vendor_words2str()
> > > due to renaming of the last in previous patch
> > > ---
> > > target-i386/cpu.c | 27 ---
> > > target-i386/cpu.h |1 -
> > > 2 files changed, 12 insertions(+), 16 deletions(-)
> > >
[...]
--
Eduardo
[Qemu-devel] [PATCH v2 1/3] block: make qiov_is_aligned() public
The qiov_is_aligned() function checks whether a QEMUIOVector meets a
BlockDriverState's alignment requirements. This is needed by
virtio-blk-data-plane so:
1. Move the function from block/raw-posix.c to block/block.c.
2. Make it public in block/block.h.
3. Rename to bdrv_qiov_is_aligned().
4. Change return type from int to bool.
Signed-off-by: Stefan Hajnoczi
---
block.c | 16
block/raw-posix.c | 18 +-
include/block/block.h | 1 +
3 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/block.c b/block.c
index 4e28c55..11ca661 100644
--- a/block.c
+++ b/block.c
@@ -4323,6 +4323,22 @@ void *qemu_blockalign(BlockDriverState *bs, size_t size)
return qemu_memalign((bs && bs->buffer_alignment) ? bs->buffer_alignment :
512, size);
}
+/*
+ * Check if all memory in this vector is sector aligned.
+ */
+bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov)
+{
+int i;
+
+for (i = 0; i < qiov->niov; i++) {
+if ((uintptr_t) qiov->iov[i].iov_base % bs->buffer_alignment) {
+return false;
+}
+}
+
+return true;
+}
+
void bdrv_set_dirty_tracking(BlockDriverState *bs, int enable)
{
int64_t bitmap_size;
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 87d888e..02d2a4e 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -430,22 +430,6 @@ static void raw_reopen_abort(BDRVReopenState *state)
#endif
*/
-/*
- * Check if all memory in this vector is sector aligned.
- */
-static int qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov)
-{
-int i;
-
-for (i = 0; i < qiov->niov; i++) {
-if ((uintptr_t) qiov->iov[i].iov_base % bs->buffer_alignment) {
-return 0;
-}
-}
-
-return 1;
-}
-
static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb)
{
int ret;
@@ -722,7 +706,7 @@ static BlockDriverAIOCB *raw_aio_submit(BlockDriverState
*bs,
* driver that it needs to copy the buffer.
*/
if ((bs->open_flags & BDRV_O_NOCACHE)) {
-if (!qiov_is_aligned(bs, qiov)) {
+if (!bdrv_qiov_is_aligned(bs, qiov)) {
type |= QEMU_AIO_MISALIGNED;
#ifdef CONFIG_LINUX_AIO
} else if (s->use_aio) {
diff --git a/include/block/block.h b/include/block/block.h
index 0719339..ffd1936 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -349,6 +349,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
void bdrv_set_buffer_alignment(BlockDriverState *bs, int align);
void *qemu_blockalign(BlockDriverState *bs, size_t size);
+bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov);
#define BDRV_SECTORS_PER_DIRTY_CHUNK 2048
--
1.8.0.2
[Qemu-devel] [PATCH v2 0/3] dataplane: misaligned buffers support for Windows guests
Windows guests do not work with x-data-plane=on because misaligned request support is missing in hw/dataplane/virtio-blk.c. This series adds a bounce buffer when the request is misaligned. Windows guests now work with x-data-plane=on. v2: * Use qemu_iovec_from_buffer() on read completion, not qemu_iovec_to_buffer() [Stefan] Stefan Hajnoczi (3): block: make qiov_is_aligned() public dataplane: extract virtio-blk read/write processing into do_rdwr_cmd() dataplane: handle misaligned virtio-blk requests block.c | 16 +++ block/raw-posix.c | 18 + hw/dataplane/virtio-blk.c | 67 +++ include/block/block.h | 1 + 4 files changed, 74 insertions(+), 28 deletions(-) -- 1.8.0.2
Re: [Qemu-devel] [PATCH 3/3] dataplane: handle misaligned virtio-blk requests
Am 10.01.2013 17:48, schrieb Stefan Hajnoczi:
> O_DIRECT on Linux has alignment requirements on I/O buffers and
> misaligned requests result in -EINVAL. The Linux virtio_blk guest
> driver usually submits aligned requests so I forgot to handle misaligned
> requests.
>
> It turns out that virtio-win guest drivers submit misaligned requests.
> Handle them using a bounce buffer that meets alignment requirements.
>
> Signed-off-by: Stefan Hajnoczi
> ---
> hw/dataplane/virtio-blk.c | 36
> 1 file changed, 36 insertions(+)
>
> diff --git a/hw/dataplane/virtio-blk.c b/hw/dataplane/virtio-blk.c
> index a6696b8..991ab5f 100644
> --- a/hw/dataplane/virtio-blk.c
> +++ b/hw/dataplane/virtio-blk.c
> @@ -34,6 +34,8 @@ typedef struct {
> struct iocb iocb; /* Linux AIO control block */
> QEMUIOVector *inhdr;/* iovecs for virtio_blk_inhdr */
> unsigned int head; /* vring descriptor index */
> +void *bounce_buffer;/* used if guest buffers are unaligned */
> +QEMUIOVector *read_qiov;/* for read completion /w bounce buffer
> */
> } VirtIOBlockRequest;
>
> struct VirtIOBlockDataPlane {
> @@ -89,6 +91,15 @@ static void complete_request(struct iocb *iocb, ssize_t
> ret, void *opaque)
>
> trace_virtio_blk_data_plane_complete_request(s, req->head, ret);
>
> +if (req->read_qiov) {
> +assert(req->bounce_buffer);
> +qemu_iovec_to_buf(req->read_qiov, 0, req->bounce_buffer, len);
Shouldn't it be qemu_iovec_from_buf()?
Makes me wonder if qtest cases might be doable...
> +qemu_iovec_destroy(req->read_qiov);
> +g_slice_free(QEMUIOVector, req->read_qiov);
> +}
> +
> +qemu_vfree(req->bounce_buffer);
> +
> qemu_iovec_from_buf(req->inhdr, 0, &hdr, sizeof(hdr));
> qemu_iovec_destroy(req->inhdr);
> g_slice_free(QEMUIOVector, req->inhdr);
> @@ -136,6 +147,29 @@ static int do_rdwr_cmd(VirtIOBlockDataPlane *s, bool
> read,
> QEMUIOVector *inhdr)
> {
> struct iocb *iocb;
> +QEMUIOVector qiov;
> +struct iovec bounce_iov;
> +void *bounce_buffer = NULL;
> +QEMUIOVector *read_qiov = NULL;
> +
> +qemu_iovec_init_external(&qiov, iov, iov_cnt);
> +if (!bdrv_qiov_is_aligned(s->blk->conf.bs, &qiov)) {
> +/* Redirect I/O to aligned bounce buffer */
> +bounce_buffer = qemu_blockalign(s->blk->conf.bs, qiov.size);
> +bounce_iov.iov_base = bounce_buffer;
> +bounce_iov.iov_len = qiov.size;
> +iov = &bounce_iov;
> +iov_cnt = 1;
> +
> +if (read) {
> +/* Need to copy back from bounce buffer on completion */
> +read_qiov = g_slice_new(QEMUIOVector);
> +qemu_iovec_init(read_qiov, iov_cnt);
> +qemu_iovec_concat_iov(read_qiov, iov, iov_cnt, 0, qiov.size);
This would have to be the original iov and iov_cnt, but they are already
overwritten here.
> +} else {
> +qemu_iovec_to_buf(&qiov, 0, bounce_buffer, qiov.size);
> +}
> +}
>
> iocb = ioq_rdwr(&s->ioqueue, read, iov, iov_cnt, offset);
>
> @@ -143,6 +177,8 @@ static int do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read,
> VirtIOBlockRequest *req = container_of(iocb, VirtIOBlockRequest, iocb);
> req->head = head;
> req->inhdr = inhdr;
> +req->bounce_buffer = bounce_buffer;
> +req->read_qiov = read_qiov;
> return 0;
> }
Kevin
Re: [Qemu-devel] [PATCH 0/3] dataplane: misaligned buffers support for Windows guests
Am 10.01.2013 17:48, schrieb Stefan Hajnoczi: > Windows guests do not work with x-data-plane=on because misaligned request > support is missing in hw/dataplane/virtio-blk.c. This series adds a bounce > buffer when the request is misaligned. Windows guests now work with > x-data-plane=on. > > Stefan Hajnoczi (3): > block: make qiov_is_aligned() public > dataplane: extract virtio-blk read/write processing into do_rdwr_cmd() > dataplane: handle misaligned virtio-blk requests > > block.c | 16 +++ > block/raw-posix.c | 18 + > hw/dataplane/virtio-blk.c | 67 > +++ > include/block/block.h | 1 + > 4 files changed, 74 insertions(+), 28 deletions(-) Patches 1 and 2: Reviewed-by: Kevin Wolf
[Qemu-devel] [PATCH v2 3/3] dataplane: handle misaligned virtio-blk requests
O_DIRECT on Linux has alignment requirements on I/O buffers and
misaligned requests result in -EINVAL. The Linux virtio_blk guest
driver usually submits aligned requests so I forgot to handle misaligned
requests.
It turns out that virtio-win guest drivers submit misaligned requests.
Handle them using a bounce buffer that meets alignment requirements.
Signed-off-by: Stefan Hajnoczi
---
hw/dataplane/virtio-blk.c | 36
1 file changed, 36 insertions(+)
diff --git a/hw/dataplane/virtio-blk.c b/hw/dataplane/virtio-blk.c
index a6696b8..88300a6 100644
--- a/hw/dataplane/virtio-blk.c
+++ b/hw/dataplane/virtio-blk.c
@@ -34,6 +34,8 @@ typedef struct {
struct iocb iocb; /* Linux AIO control block */
QEMUIOVector *inhdr;/* iovecs for virtio_blk_inhdr */
unsigned int head; /* vring descriptor index */
+void *bounce_buffer;/* used if guest buffers are unaligned */
+QEMUIOVector *read_qiov;/* for read completion /w bounce buffer */
} VirtIOBlockRequest;
struct VirtIOBlockDataPlane {
@@ -89,6 +91,15 @@ static void complete_request(struct iocb *iocb, ssize_t ret,
void *opaque)
trace_virtio_blk_data_plane_complete_request(s, req->head, ret);
+if (req->read_qiov) {
+assert(req->bounce_buffer);
+qemu_iovec_from_buf(req->read_qiov, 0, req->bounce_buffer, len);
+qemu_iovec_destroy(req->read_qiov);
+g_slice_free(QEMUIOVector, req->read_qiov);
+}
+
+qemu_vfree(req->bounce_buffer);
+
qemu_iovec_from_buf(req->inhdr, 0, &hdr, sizeof(hdr));
qemu_iovec_destroy(req->inhdr);
g_slice_free(QEMUIOVector, req->inhdr);
@@ -136,6 +147,29 @@ static int do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read,
QEMUIOVector *inhdr)
{
struct iocb *iocb;
+QEMUIOVector qiov;
+struct iovec bounce_iov;
+void *bounce_buffer = NULL;
+QEMUIOVector *read_qiov = NULL;
+
+qemu_iovec_init_external(&qiov, iov, iov_cnt);
+if (!bdrv_qiov_is_aligned(s->blk->conf.bs, &qiov)) {
+/* Redirect I/O to aligned bounce buffer */
+bounce_buffer = qemu_blockalign(s->blk->conf.bs, qiov.size);
+bounce_iov.iov_base = bounce_buffer;
+bounce_iov.iov_len = qiov.size;
+iov = &bounce_iov;
+iov_cnt = 1;
+
+if (read) {
+/* Need to copy back from bounce buffer on completion */
+read_qiov = g_slice_new(QEMUIOVector);
+qemu_iovec_init(read_qiov, iov_cnt);
+qemu_iovec_concat_iov(read_qiov, iov, iov_cnt, 0, qiov.size);
+} else {
+qemu_iovec_to_buf(&qiov, 0, bounce_buffer, qiov.size);
+}
+}
iocb = ioq_rdwr(&s->ioqueue, read, iov, iov_cnt, offset);
@@ -143,6 +177,8 @@ static int do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read,
VirtIOBlockRequest *req = container_of(iocb, VirtIOBlockRequest, iocb);
req->head = head;
req->inhdr = inhdr;
+req->bounce_buffer = bounce_buffer;
+req->read_qiov = read_qiov;
return 0;
}
--
1.8.0.2
[Qemu-devel] [PATCH v2 2/3] dataplane: extract virtio-blk read/write processing into do_rdwr_cmd()
Extract code for read/write command processing into do_rdwr_cmd(). This
brings together pieces that are spread across process_request().
The real motivation is to set the stage for handling misaligned
requests, which the next patch tackles.
Signed-off-by: Stefan Hajnoczi
---
hw/dataplane/virtio-blk.c | 31 ---
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/hw/dataplane/virtio-blk.c b/hw/dataplane/virtio-blk.c
index 4c4ad84..a6696b8 100644
--- a/hw/dataplane/virtio-blk.c
+++ b/hw/dataplane/virtio-blk.c
@@ -130,6 +130,22 @@ static void do_get_id_cmd(VirtIOBlockDataPlane *s,
complete_request_early(s, head, inhdr, VIRTIO_BLK_S_OK);
}
+static int do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read,
+ struct iovec *iov, unsigned int iov_cnt,
+ long long offset, unsigned int head,
+ QEMUIOVector *inhdr)
+{
+struct iocb *iocb;
+
+iocb = ioq_rdwr(&s->ioqueue, read, iov, iov_cnt, offset);
+
+/* Fill in virtio block metadata needed for completion */
+VirtIOBlockRequest *req = container_of(iocb, VirtIOBlockRequest, iocb);
+req->head = head;
+req->inhdr = inhdr;
+return 0;
+}
+
static int process_request(IOQueue *ioq, struct iovec iov[],
unsigned int out_num, unsigned int in_num,
unsigned int head)
@@ -139,7 +155,6 @@ static int process_request(IOQueue *ioq, struct iovec iov[],
struct virtio_blk_outhdr outhdr;
QEMUIOVector *inhdr;
size_t in_size;
-struct iocb *iocb;
/* Copy in outhdr */
if (unlikely(iov_to_buf(iov, out_num, 0, &outhdr,
@@ -167,12 +182,12 @@ static int process_request(IOQueue *ioq, struct iovec
iov[],
switch (outhdr.type) {
case VIRTIO_BLK_T_IN:
-iocb = ioq_rdwr(ioq, true, in_iov, in_num, outhdr.sector * 512);
-break;
+do_rdwr_cmd(s, true, in_iov, in_num, outhdr.sector * 512, head, inhdr);
+return 0;
case VIRTIO_BLK_T_OUT:
-iocb = ioq_rdwr(ioq, false, iov, out_num, outhdr.sector * 512);
-break;
+do_rdwr_cmd(s, false, iov, out_num, outhdr.sector * 512, head, inhdr);
+return 0;
case VIRTIO_BLK_T_SCSI_CMD:
/* TODO support SCSI commands */
@@ -198,12 +213,6 @@ static int process_request(IOQueue *ioq, struct iovec
iov[],
g_slice_free(QEMUIOVector, inhdr);
return -EFAULT;
}
-
-/* Fill in virtio block metadata needed for completion */
-VirtIOBlockRequest *req = container_of(iocb, VirtIOBlockRequest, iocb);
-req->head = head;
-req->inhdr = inhdr;
-return 0;
}
static void handle_notify(EventHandler *handler)
--
1.8.0.2
Re: [Qemu-devel] [Qemu-trivial] [PATCH] configure: try pkg-config for curses
ping
Пнд 31 Дек 2012 20:30:59, Vadim Evard писал:
configure: try pkg-config for curses
Static linkikng against ncurses may require explicit -ltinfo.
In case -lcurses and -lncurses both didn't work give pkg-config a
chance.
Signed-off-by: Vadim Evard
---
configure | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index b0c7e54..16280e2 100755
--- a/configure
+++ b/configure
@@ -2030,7 +2030,7 @@ fi
if test "$mingw32" = "yes" ; then
curses_list="-lpdcurses"
else
- curses_list="-lncurses -lcurses"
+ curses_list="-lncurses:-lcurses:$($pkg_config --libs ncurses)"
fi
if test "$curses" != "no" ; then
@@ -2043,7 +2043,9 @@ int main(void) {
return s != 0;
}
EOF
+ IFS=:
for curses_lib in $curses_list; do
+ unset IFS
if compile_prog "" "$curses_lib" ; then
curses_found=yes
libs_softmmu="$curses_lib $libs_softmmu"
Re: [Qemu-devel] [PATCH] block: fix initialization in bdrv_io_limits_enable()
Il 11/01/2013 13:29, Peter Lieven ha scritto:
> bdrv_io_limits_enable() starts a new slice, but does not set io_base
> correctly for that slice.
>
> Here is how io_base is used:
>
>bytes_base = bs->nr_bytes[is_write] - bs->io_base.bytes[is_write];
>bytes_res = (unsigned) nb_sectors * BDRV_SECTOR_SIZE;
>
>if (bytes_base + bytes_res <= bytes_limit) {
>/* no wait */
>} else {
>/* operation needs to be throttled */
>}
>
> As a result, any I/O operations that are triggered between now and
> bs->slice_end are incorrectly limited. If 10 MB of data has been
> written since the VM was started, QEMU thinks that 10 MB of data has
> been written in this slice. This leads to a I/O lockup in the guest.
>
> We fix this by delaying the start of a new slice to the next
> call of bdrv_exceed_io_limits().
>
> Signed-off-by: Peter Lieven
> ---
> block.c |4
> 1 file changed, 4 deletions(-)
>
> diff --git a/block.c b/block.c
> index 4e28c55..a40a389 100644
> --- a/block.c
> +++ b/block.c
> @@ -155,10 +155,6 @@ void bdrv_io_limits_enable(BlockDriverState *bs)
> {
> qemu_co_queue_init(&bs->throttled_reqs);
> bs->block_timer = qemu_new_timer_ns(vm_clock, bdrv_block_timer, bs);
> -bs->slice_time = 5 * BLOCK_IO_SLICE_TIME;
> -bs->slice_start = qemu_get_clock_ns(vm_clock);
> -bs->slice_end = bs->slice_start + bs->slice_time;
> -memset(&bs->io_base, 0, sizeof(bs->io_base));
> bs->io_limits_enabled = true;
> }
>
Thanks!
Reviewed-by: Paolo Bonzini
Paolo
[Qemu-devel] [Bug 1025244] Re: qcow2 image increasing disk size above the virtual limit
Thanks for your advices. I have no more problems with VM-size since deleting snapshot in shutdown-mode. I reduced the overlarge qcow2-images by converting in qcow2 again (that detects unused sectors and omits this). -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1025244 Title: qcow2 image increasing disk size above the virtual limit Status in QEMU: New Status in “qemu-kvm” package in Ubuntu: Triaged Bug description: Using qemu/kvm, qcow2 images, ext4 file systems on both guest and host Host and Guest: Ubuntu server 12.04 64bit To create an image I did this: qemu-img create -f qcow2 -o preallocation=metadata ubuntu-pdc-vda.img 10737418240 (not sure about the exact bytes, but around this) ls -l ubuntu-pdc-vda.img fallocate -l theSizeInBytesFromAbove ubuntu-pdc-vda.img The problem is that the image is growing progressively and has obviously no limit, although I gave it one. The root filesystem's image is the same case: qemu-img info ubuntu-pdc-vda.img image: ubuntu-pdc-vda.img file format: qcow2 virtual size: 10G (10737418240 bytes) disk size: 14G cluster_size: 65536 and for confirmation: du -sh ubuntu-pdc-vda.img 15G ubuntu-pdc-vda.img I made a test and saw that when I delete something from the guest, the real size of the image is not decreasing (I read it is normal). OK, but when I write something again, it doesn't use the freed space, but instead grows the image. So for example: 1. The initial physical size of the image is 1GB. 2. I copy 1GB of data in the guest. It's physical size becomes 2GB. 3. I delete this data (1GB). The physical size of the image remains 2GB. 4. I copy another 1GB of data to the guest. 5. The physical size of the image becomes 3GB. 6. And so on with no limit. It doesn't care if the virtual size is less. Is this normal - the real/physical size of the image to be larger than the virtual limit??? To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1025244/+subscriptions
Re: [Qemu-devel] [PATCH 3/3] dataplane: handle misaligned virtio-blk requests
Il 11/01/2013 13:39, Kevin Wolf ha scritto:
> Am 10.01.2013 17:48, schrieb Stefan Hajnoczi:
>> O_DIRECT on Linux has alignment requirements on I/O buffers and
>> misaligned requests result in -EINVAL. The Linux virtio_blk guest
>> driver usually submits aligned requests so I forgot to handle misaligned
>> requests.
>>
>> It turns out that virtio-win guest drivers submit misaligned requests.
>> Handle them using a bounce buffer that meets alignment requirements.
>>
>> Signed-off-by: Stefan Hajnoczi
>> ---
>> hw/dataplane/virtio-blk.c | 36
>> 1 file changed, 36 insertions(+)
>>
>> diff --git a/hw/dataplane/virtio-blk.c b/hw/dataplane/virtio-blk.c
>> index a6696b8..991ab5f 100644
>> --- a/hw/dataplane/virtio-blk.c
>> +++ b/hw/dataplane/virtio-blk.c
>> @@ -34,6 +34,8 @@ typedef struct {
>> struct iocb iocb; /* Linux AIO control block */
>> QEMUIOVector *inhdr;/* iovecs for virtio_blk_inhdr */
>> unsigned int head; /* vring descriptor index */
>> +void *bounce_buffer;/* used if guest buffers are unaligned
>> */
>> +QEMUIOVector *read_qiov;/* for read completion /w bounce buffer
>> */
>> } VirtIOBlockRequest;
>>
>> struct VirtIOBlockDataPlane {
>> @@ -89,6 +91,15 @@ static void complete_request(struct iocb *iocb, ssize_t
>> ret, void *opaque)
>>
>> trace_virtio_blk_data_plane_complete_request(s, req->head, ret);
>>
>> +if (req->read_qiov) {
>> +assert(req->bounce_buffer);
>> +qemu_iovec_to_buf(req->read_qiov, 0, req->bounce_buffer, len);
>
> Shouldn't it be qemu_iovec_from_buf()?
Yes.
> Makes me wonder if qtest cases might be doable...
Anthony, what's going on with libqos? :)
In the meanwhile...
>> +qemu_iovec_destroy(req->read_qiov);
>> +g_slice_free(QEMUIOVector, req->read_qiov);
>> +}
>> +
>> +qemu_vfree(req->bounce_buffer);
>> +
>> qemu_iovec_from_buf(req->inhdr, 0, &hdr, sizeof(hdr));
>> qemu_iovec_destroy(req->inhdr);
>> g_slice_free(QEMUIOVector, req->inhdr);
>> @@ -136,6 +147,29 @@ static int do_rdwr_cmd(VirtIOBlockDataPlane *s, bool
>> read,
>> QEMUIOVector *inhdr)
>> {
>> struct iocb *iocb;
>> +QEMUIOVector qiov;
>> +struct iovec bounce_iov;
>> +void *bounce_buffer = NULL;
>> +QEMUIOVector *read_qiov = NULL;
>> +
>> +qemu_iovec_init_external(&qiov, iov, iov_cnt);
>> +if (!bdrv_qiov_is_aligned(s->blk->conf.bs, &qiov)) {
... changing this to "if (1)" would be enough for testing with Linux guests.
Paolo
>> +/* Redirect I/O to aligned bounce buffer */
>> +bounce_buffer = qemu_blockalign(s->blk->conf.bs, qiov.size);
>> +bounce_iov.iov_base = bounce_buffer;
>> +bounce_iov.iov_len = qiov.size;
>> +iov = &bounce_iov;
>> +iov_cnt = 1;
>> +
>> +if (read) {
>> +/* Need to copy back from bounce buffer on completion */
>> +read_qiov = g_slice_new(QEMUIOVector);
>> +qemu_iovec_init(read_qiov, iov_cnt);
>> +qemu_iovec_concat_iov(read_qiov, iov, iov_cnt, 0, qiov.size);
>
> This would have to be the original iov and iov_cnt, but they are already
> overwritten here.
>
>> +} else {
>> +qemu_iovec_to_buf(&qiov, 0, bounce_buffer, qiov.size);
>> +}
>> +}
>>
>> iocb = ioq_rdwr(&s->ioqueue, read, iov, iov_cnt, offset);
>>
>> @@ -143,6 +177,8 @@ static int do_rdwr_cmd(VirtIOBlockDataPlane *s, bool
>> read,
>> VirtIOBlockRequest *req = container_of(iocb, VirtIOBlockRequest, iocb);
>> req->head = head;
>> req->inhdr = inhdr;
>> +req->bounce_buffer = bounce_buffer;
>> +req->read_qiov = read_qiov;
>> return 0;
>> }
>
> Kevin
>
>
Re: [Qemu-devel] [PATCH v2 3/3] dataplane: handle misaligned virtio-blk requests
Il 11/01/2013 13:34, Stefan Hajnoczi ha scritto:
> +iov = &bounce_iov;
> +iov_cnt = 1;
> +
> +if (read) {
> +/* Need to copy back from bounce buffer on completion */
> +read_qiov = g_slice_new(QEMUIOVector);
> +qemu_iovec_init(read_qiov, iov_cnt);
> +qemu_iovec_concat_iov(read_qiov, iov, iov_cnt, 0, qiov.size);
This is still wrong, how did you test it?
Paolo
Re: [Qemu-devel] [PATCH V8 2/5] Adding utility function net_checksum_add_iov() for iovec checksum calculation
Thanks for pointing out, Stefan.
I've rebased the code and fixed these issues.
On Mon, Jan 7, 2013 at 4:04 PM, Stefan Hajnoczi wrote:
> On Fri, Dec 07, 2012 at 01:15:06PM +0200, Dmitry Fleytman wrote:
> > Signed-off-by: Dmitry Fleytman
> > Signed-off-by: Yan Vugenfirer
> > ---
> > iov.h | 5 +
> > net/checksum.c | 28
> > net/checksum.h | 8
> > 3 files changed, 41 insertions(+)
> >
> > diff --git a/iov.h b/iov.h
> > index 34c8ec9..c184a80 100644
> > --- a/iov.h
> > +++ b/iov.h
> > @@ -11,6 +11,9 @@
>
> This file has been moved in qemu.git/master. It is now
> include/qemu/iov.h.
>
> > * the COPYING file in the top-level directory.
> > */
> >
> > +#ifndef QEMU_IOV_H
> > +#define QEMU_IOV_H
> > +
>
> This is already present in qemu.git/master.
>
> > #include "qemu-common.h"
> >
> > /**
> > @@ -95,3 +98,5 @@ void iov_hexdump(const struct iovec *iov, const
> unsigned int iov_cnt,
> > unsigned iov_copy(struct iovec *dst_iov, unsigned int dst_iov_cnt,
> > const struct iovec *iov, unsigned int iov_cnt,
> > size_t offset, size_t bytes);
> > +
> > +#endif /* QEMU_IOV_H */
>
> Same here. Just pointing out expected rebase conflicts you'll see for
> the next revision of this series.
>
> > diff --git a/net/checksum.c b/net/checksum.c
> > index 4fa5563..9c813ff 100644
> > --- a/net/checksum.c
> > +++ b/net/checksum.c
> > @@ -84,3 +84,31 @@ void net_checksum_calculate(uint8_t *data, int length)
> > data[14+hlen+csum_offset] = csum >> 8;
> > data[14+hlen+csum_offset+1] = csum & 0xff;
> > }
> > +
> > +uint32_t
> > +net_checksum_add_iov(const struct iovec *iov, const unsigned int
> iov_cnt,
> > + uint32_t iov_off, uint32_t size)
> > +{
> > +size_t iovec_off, buf_off;
> > +unsigned int i;
> > +uint32_t res = 0;
> > +uint32_t seq = 0;
> > +
> > +iovec_off = 0;
> > +buf_off = 0;
> > +for (i = 0; i < iov_cnt && size; i++) {
> > +if (iov_off < (iovec_off + iov[i].iov_len)) {
> > +size_t len = MIN((iovec_off + iov[i].iov_len) - iov_off ,
> size);
> > +void *chunk_buf = iov[i].iov_base + (iov_off - iovec_off);
> > +
> > +res += net_checksum_add_cont(len, chunk_buf, seq);
> > +seq += len;
> > +
> > +buf_off += len;
> > +iov_off += len;
> > +size -= len;
> > +}
> > +iovec_off += iov[i].iov_len;
> > +}
> > +return res;
> > +}
> > diff --git a/net/checksum.h b/net/checksum.h
> > index 171924c..e63c482 100644
> > --- a/net/checksum.h
> > +++ b/net/checksum.h
> > @@ -19,6 +19,7 @@
> > #define QEMU_NET_CHECKSUM_H
> >
> > #include
> > +#include "iov.h"
>
> iov.h is the wrong header file, you need struct iovec from
> qemu-common.h.
>
--
Dmitry Fleytman
Technology Expert and Consultant,
Daynix Computing Ltd.
Cell: +972-54-2819481
Skype: dmitry.fleytman
Re: [Qemu-devel] [PATCH V8 3/5] Adding common definitions for VMWARE devices
Stefan, We have vmxnet2 implementation ready for submission as well, so this code is not dead :) Dmitry. On Mon, Jan 7, 2013 at 4:17 PM, Stefan Hajnoczi wrote: > On Fri, Dec 07, 2012 at 01:15:07PM +0200, Dmitry Fleytman wrote: > > diff --git a/hw/vmxnet_debug.h b/hw/vmxnet_debug.h > > new file mode 100644 > > index 000..faa1431 > > --- /dev/null > > +++ b/hw/vmxnet_debug.h > > @@ -0,0 +1,121 @@ > > +/* > > + * QEMU VMWARE VMXNET* paravirtual NICs - debugging facilities > > + * > > + * Copyright (c) 2012 Ravello Systems LTD (http://ravellosystems.com) > > + * > > + * Developed by Daynix Computing LTD (http://www.daynix.com) > > + * > > + * Authors: > > + * Dmitry Fleytman > > + * Tamir Shomer > > + * Yan Vugenfirer > > + * > > + * This work is licensed under the terms of the GNU GPL, version 2 or > later. > > + * See the COPYING file in the top-level directory. > > + * > > + */ > > + > > +#ifndef _QEMU_VMXNET_DEBUG_H > > +#define _QEMU_VMXNET_DEBUG_H > > + > > +#ifdef VMXNET_VERSION_2 > > +#define VMXNET_DEVICE_NAME "vmxnet" > > +#elif defined VMXNET_VERSION_3 > > +#define VMXNET_DEVICE_NAME "vmxnet3" > > +#else > > +#error "VMXNET version is not defined" > > +#endif > > Please drop the VMXNET_VERSION_* conditional dead code, only version 3 > is used. > -- Dmitry Fleytman Technology Expert and Consultant, Daynix Computing Ltd. Cell: +972-54-2819481 Skype: dmitry.fleytman
Re: [Qemu-devel] [PATCH V2 00/10] snapshot: take block snapshots in unified way
On Thu, 10 Jan 2013 14:01:27 +0800 Wenchao Xia wrote: > 于 2013-1-10 6:34, Eric Blake 写道: > > On 01/07/2013 12:27 AM, Wenchao Xia wrote: > >>These patch added a seperated layer to take internal or external > >> snapshots > >> in a unified way, the granularity is block device, so other functions can > >> just combine the request and submit, such as group snapshot, savevm. > >> > >> Total goal are: > >>Live back up vm in external or internal image, which need three > >> functions: > >> 1 live snapshot block device internal/external. > >> 2 live save vmstate internal/external. > >> 3 combination of the function unit. > >> > >>This serial provide part one. > >> > > > > The design on this series needs to be coordinated with Pavel's > > attempts[1] to convert the existing HMP snapshot commands into QMP. I > > don't want to spend too much time reviewing two different divergent > > designs that both have the same goal of finally managing snapshots under > > QMP. > > > > [1] https://lists.gnu.org/archive/html/qemu-devel/2013-01/msg01326.html > > >A bit difference: This serial are taking internal/external block > snapshots but Pavel's are focusing are vm whole internal snapshots. It Pavel's work makes savevm, loadvm and delvm available in QMP. Also note that we do have blockdev-snapshot-sync. How does this series relate to it? I don't know if it's just me but, honestly speaking, I'm getting confused about the various proposals and APIs for snapshots. IMHO, it's time to go back a bit from code and discuss the status of the current APIs, what's missing and what the proposals are. This should also include writing a wiki page, as Anthony has done for previous ideas (which worked quite well).
Re: [Qemu-devel] [PATCH 2/3] vnc: added initial websocket protocol support
On 01/08/2013 01:38 AM, Anthony Liguori wrote: > Better, but I still think it's better to advance the buffer based on the > parsed header sizes that to assume there's no additional data. I have used buffer_advance in the patch set v6. Does anything else need to be changed? Regards Tim -- SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg) Maxfeldstr. 5, 90409 Nürnberg, Germany T: +49 (0) 911 74053-0 F: +49 (0) 911 74053-483 http://www.suse.de/ signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH 2/2] qbus: make bus reset overrideable by subclasses
Signed-off-by: Anthony Liguori
---
hw/qdev-core.h | 15 +++
hw/qdev.c | 16 +++-
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/hw/qdev-core.h b/hw/qdev-core.h
index f40fd15..453a061 100644
--- a/hw/qdev-core.h
+++ b/hw/qdev-core.h
@@ -100,7 +100,22 @@ struct BusClass {
* bindings can be found at http://playground.sun.com/1275/bindings/.
*/
char *(*get_fw_dev_path)(DeviceState *dev);
+
+/**
+ * reset:
+ *
+ * Cold reset of a bus. This only resets the controller state of the bus.
+ */
int (*reset)(BusState *bus);
+
+/**
+ * reset_all:
+ *
+ * Cold reset of a bus with children. The default implementation of this
+ * method will invoke BusState::reset and then recursively call
+ * qdev_reset_all() on each child in an arbitrary order.
+ */
+void (*reset_all)(BusState *bus);
};
typedef struct BusChild {
diff --git a/hw/qdev.c b/hw/qdev.c
index e02b5be..db19b14 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -234,11 +234,17 @@ void qdev_reset_all(DeviceState *dev)
dc->reset_all(dev);
}
-void qbus_reset_all(BusState *bus)
+static void qbus_reset_children(BusState *bus)
{
qbus_walk_children(bus, qdev_reset_one, qbus_reset_one, NULL);
}
+void qbus_reset_all(BusState *bus)
+{
+BusClass *bc = BUS_GET_CLASS(bus);
+bc->reset_all(bus);
+}
+
void qbus_reset_all_fn(void *opaque)
{
BusState *bus = opaque;
@@ -758,6 +764,13 @@ static const TypeInfo device_type_info = {
.class_size = sizeof(DeviceClass),
};
+static void qbus_class_initfn(ObjectClass *klass, void *data)
+{
+BusClass *bc = BUS_CLASS(klass);
+
+bc->reset_all = qbus_reset_children;
+}
+
static void qbus_initfn(Object *obj)
{
BusState *bus = BUS(obj);
@@ -789,6 +802,7 @@ static const TypeInfo bus_info = {
.parent = TYPE_OBJECT,
.instance_size = sizeof(BusState),
.abstract = true,
+.class_init = qbus_class_initfn,
.class_size = sizeof(BusClass),
.instance_init = qbus_initfn,
.instance_finalize = qbus_finalize,
--
1.8.0
[Qemu-devel] [PATCH 0/2] qdev: make reset propagation overrideable by subclasses
This series allows subclasses to override how reset propagates to child devices and/or busses. This will allow a controller to perform specific actions either before or after reset occurs for a specific device. Since DeviceState::reset should model cold reset, I don't believe that any controller would really need to override propagation order but I think its useful to demonstrate how a bus specific reset function should be implemented. This applies on top of Paolo's virtio-scsi reset fix patch series.
