Re: [Xen-devel] [Qemu-devel] [PATCH v3 01/46] Replace all occurances of __FUNCTION__ with __func__
On 11/08/2017 08:51 AM, Alistair Francis wrote: >>>> Let me rephrase the question: do we really support compilers that don't >>>> understand __func__? The presence of numerous unconditional uses of >>>> __func__ in the tree means the answer is no. Let's replace AUDIO_FUNC >>>> by plain __func__. >>> >>> Answered elsewhere in patch 3/46 (where we DO replace AUDIO_FUNC by >>> __func__). >> >> I see. >> >> Put 03/46 first, so we don't have to mess with AUDIO_FUNC twice? > > I would really like to avoid that, as the conflicts will be a bit of a > mess. The way I see it there will be a lot of churn no matter what, > so we don't gain much by swapping the order around. > > I have a new series ready to send today, so I'm going to send that > through as I would like at least some of these patches to make it in > 2.11. After that if you think strongly the order should be changed I > can change it in the next version. I think the reorder is not that hard. Put 3/46 first (changing AUDIO_FUNC to __func__), and then 1/46 doesn't have to touch any of the files that used to use AUDIO_FUNC, because there is no intermediate state using __FUNCTION__. If I'm reading it correctly, the rebase conflict is limited to a slight rewording of the commit message for 3/46, and one line in 1/46 to the definition of AUDIO_FUNC (that will no longer be present). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH v3 01/46] Replace all occurances of __FUNCTION__ with __func__
On 11/07/2017 04:12 AM, Markus Armbruster wrote: > Juan Quintela <quint...@redhat.com> writes: > >> Alistair Francis <alistair.fran...@xilinx.com> wrote: >>> Replace all occurs of __FUNCTION__ except for the check in checkpatch >>> with the non GCC specific __func__. >>> >>> +++ b/audio/audio_int.h >>> @@ -253,7 +253,7 @@ static inline int audio_ring_dist (int dst, int src, >>> int len) >>> #define AUDIO_STRINGIFY(n) AUDIO_STRINGIFY_(n) >>> >>> #if defined _MSC_VER || defined __GNUC__ >>> -#define AUDIO_FUNC __FUNCTION__ >>> +#define AUDIO_FUNC __func__ >>> #else >>> #define AUDIO_FUNC __FILE__ ":" AUDIO_STRINGIFY (__LINE__) >>> #endif >> >> Unrelated to this patch >> Do we really support other compilers than msc and gcc? > > Let me rephrase the question: do we really support compilers that don't > understand __func__? The presence of numerous unconditional uses of > __func__ in the tree means the answer is no. Let's replace AUDIO_FUNC > by plain __func__. Answered elsewhere in patch 3/46 (where we DO replace AUDIO_FUNC by __func__). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH v1 1/8] Replace all occurances of __FUNCTION__ with __func__
+1312,7 @@ static void omap_prcm_apll_update(struct omap_prcm_s *s)
>
> if (mode[0] == 1 || mode[0] == 2 || mode[1] == 1 || mode[1] == 2)
> fprintf(stderr, "%s: bad EN_54M_PLL or bad EN_96M_PLL\n",
> -__FUNCTION__);
> +__func__);
More of the same. I'll quit pointing it out.
> +++ b/hw/block/onenand.c
> @@ -661,12 +661,12 @@ static uint64_t onenand_read(void *opaque, hwaddr addr,
> case 0xff02: /* ECC Result of spare area data */
> case 0xff03: /* ECC Result of main area data */
> case 0xff04: /* ECC Result of spare area data */
> -hw_error("%s: imeplement ECC\n", __FUNCTION__);
> +hw_error("%s: imeplement ECC\n", __func__);
Should we fix the typo while here? s/imeplement/implement/
> +++ b/hw/isa/vt82c686.c
> @@ -30,7 +30,7 @@
> //#define DEBUG_VT82C686B
>
> #ifdef DEBUG_VT82C686B
> -#define DPRINTF(fmt, ...) fprintf(stderr, "%s: " fmt, __FUNCTION__,
> ##__VA_ARGS__)
> +#define DPRINTF(fmt, ...) fprintf(stderr, "%s: " fmt, __func__,
> ##__VA_ARGS__)
> #else
> #define DPRINTF(fmt, ...)
> #endif
Not this patch's problem, but I hate bit-rottable statements. This
should be fixed separately into a form that always evaluates under
-Wformat (guarded by an if(0) in normal builds).
> diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> index e8b2eef688..41a7690560 100644
> --- a/hw/mips/gt64xxx_pci.c
> +++ b/hw/mips/gt64xxx_pci.c
> @@ -33,7 +33,7 @@
> //#define DEBUG
>
> #ifdef DEBUG
> -#define DPRINTF(fmt, ...) fprintf(stderr, "%s: " fmt, __FUNCTION__,
> ##__VA_ARGS__)
> +#define DPRINTF(fmt, ...) fprintf(stderr, "%s: " fmt, __func__,
> ##__VA_ARGS__)
> #else
> #define DPRINTF(fmt, ...)
> #endif
Ditto.
> +++ b/hw/usb/hcd-musb.c
> @@ -253,8 +253,8 @@
> /* #define MUSB_DEBUG */
>
> #ifdef MUSB_DEBUG
> -#define TRACE(fmt,...) fprintf(stderr, "%s@%d: " fmt "\n", __FUNCTION__, \
> - __LINE__, ##__VA_ARGS__)
> +#define TRACE(fmt, ...) fprintf(stderr, "%s@%d: " fmt "\n", __func__, \
> +__LINE__, ##__VA_ARGS__)
> #else
> #define TRACE(...)
> #endif
and again
My comments were either about things for separate patches, or things
that are trivial if you choose to touch them up, so:
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH v2] xen-disk: use g_new0 to fix build
On 07/28/2017 08:11 AM, Olaf Hering wrote:
> g_malloc0_n is available since glib-2.24. To allow build with older glib
s/is/is only/
> versions use the generic g_new0, which is already used in many other
> places in the code.
>
> Fixes commit 3284fad728 ("xen-disk: add support for multi-page shared rings")
>
> Signed-off-by: Olaf Hering <o...@aepfle.de>
> ---
> hw/block/xen_disk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] xen-disk: use g_malloc0 to fix build
On 07/28/2017 07:48 AM, Olaf Hering wrote: > On Fri, Jul 28, Eric Blake wrote: > >> This version is prone to multiplication overflow (well, maybe not, but >> you have to audit for that). Wouldn't it be better to use: > > What could go wrong? > qemu will die either way, I think. Dying immediately due to provable multiplication overflow is MUCH better than successfully allocating too-little and then having who-knows-what go wrong down the road because you didn't check for overflow. The latter can sometimes be exploited into CVEs. And maybe you can't overflow, but having to do a non-local audit to prove that is more time spent than just using the right interface from the get-go. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] xen-disk: use g_malloc0 to fix build
On 07/28/2017 07:31 AM, Olaf Hering wrote:
> g_malloc0_n is available since glib-2.24. To allow build with older glib
> versions use the generic g_malloc0, which is already used in many other
> places in the code.
>
> Fixes commit 3284fad728 ("xen-disk: add support for multi-page shared rings")
>
> Signed-off-by: Olaf Hering <o...@aepfle.de>
> ---
> hw/block/xen_disk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index d42ed7070d..71deec17b0 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -1232,7 +1232,7 @@ static int blk_connect(struct XenDevice *xendev)
> return -1;
> }
>
> -domids = g_malloc0_n(blkdev->nr_ring_ref, sizeof(uint32_t));
> +domids = g_malloc0(blkdev->nr_ring_ref * sizeof(uint32_t));
This version is prone to multiplication overflow (well, maybe not, but
you have to audit for that). Wouldn't it be better to use:
domids = g_new0(blkdev->nr_ring_ref, uint32_t)
which preserves the safety of g_malloc0_n?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v9 4/5] shutdown: Add source information to SHUTDOWN and RESET
Time to wire up all the call sites that request a shutdown or
reset to use the enum added in the previous patch.
It would have been less churn to keep the common case with no
arguments as meaning guest-triggered, and only modified the
host-triggered code paths, via a wrapper function, but then we'd
still have to audit that I didn't miss any host-triggered spots;
changing the signature forces us to double-check that I correctly
categorized all callers.
Since command line options can change whether a guest reset request
causes an actual reset vs. a shutdown, it's easy to also add the
information to reset requests.
Signed-off-by: Eric Blake <ebl...@redhat.com>
Acked-by: David Gibson <da...@gibson.dropbear.id.au> [ppc parts]
Reviewed-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk> [SPARC part]
Reviewed-by: Cornelia Huck <cornelia.h...@de.ibm.com> [s390x parts]
---
v8: rebase later in series
v7: no change
v6: defer event additions to later, add reviews of unchanged portions
v5: drop accidental addition of unrelated files
v4: s/ShutdownType/ShutdownCause/, no thanks to mingw header pollution
v3: retitle again, fix qemu-iotests, use enum rather than raw bool
in all callers
v2: retitle (was "event: Add signal information to SHUTDOWN"),
completely rework to post bool based on whether it is guest-initiated
v1: initial submission, exposing just Unix signals from host
---
include/sysemu/sysemu.h | 4 ++--
vl.c| 18 --
hw/acpi/core.c | 4 ++--
hw/arm/highbank.c | 4 ++--
hw/arm/integratorcp.c | 2 +-
hw/arm/musicpal.c | 2 +-
hw/arm/omap1.c | 10 ++
hw/arm/omap2.c | 2 +-
hw/arm/spitz.c | 2 +-
hw/arm/stellaris.c | 2 +-
hw/arm/tosa.c | 2 +-
hw/i386/pc.c| 2 +-
hw/i386/xen/xen-hvm.c | 2 +-
hw/input/pckbd.c| 4 ++--
hw/ipmi/ipmi.c | 4 ++--
hw/isa/lpc_ich9.c | 2 +-
hw/mips/boston.c| 2 +-
hw/mips/mips_malta.c| 2 +-
hw/mips/mips_r4k.c | 4 ++--
hw/misc/arm_sysctl.c| 8
hw/misc/cbus.c | 2 +-
hw/misc/macio/cuda.c| 4 ++--
hw/misc/slavio_misc.c | 4 ++--
hw/misc/zynq_slcr.c | 2 +-
hw/pci-host/apb.c | 4 ++--
hw/pci-host/bonito.c| 2 +-
hw/pci-host/piix.c | 2 +-
hw/ppc/e500.c | 2 +-
hw/ppc/mpc8544_guts.c | 2 +-
hw/ppc/ppc.c| 2 +-
hw/ppc/ppc405_uc.c | 2 +-
hw/ppc/spapr_hcall.c| 2 +-
hw/ppc/spapr_rtas.c | 4 ++--
hw/s390x/ipl.c | 2 +-
hw/sh4/r2d.c| 2 +-
hw/timer/etraxfs_timer.c| 2 +-
hw/timer/m48t59.c | 4 ++--
hw/timer/milkymist-sysctl.c | 4 ++--
hw/timer/pxa2xx_timer.c | 2 +-
hw/watchdog/watchdog.c | 2 +-
hw/xenpv/xen_domainbuild.c | 2 +-
hw/xtensa/xtfpga.c | 2 +-
kvm-all.c | 6 +++---
os-win32.c | 2 +-
qmp.c | 4 ++--
replay/replay.c | 4 ++--
target/alpha/sys_helper.c | 4 ++--
target/arm/psci.c | 4 ++--
target/i386/excp_helper.c | 2 +-
target/i386/hax-all.c | 6 +++---
target/i386/helper.c| 2 +-
target/i386/kvm.c | 2 +-
target/s390x/helper.c | 2 +-
target/s390x/kvm.c | 4 ++--
target/s390x/misc_helper.c | 4 ++--
target/sparc/int32_helper.c | 2 +-
ui/sdl.c| 2 +-
ui/sdl2.c | 4 ++--
trace-events| 2 +-
ui/cocoa.m | 2 +-
60 files changed, 98 insertions(+), 98 deletions(-)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 52102fd..e540e6f 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -62,13 +62,13 @@ typedef enum WakeupReason {
QEMU_WAKEUP_REASON_OTHER,
} WakeupReason;
-void qemu_system_reset_request(void);
+void qemu_system_reset_request(ShutdownCause reason);
void qemu_system_suspend_request(void);
void qemu_register_suspend_notifier(Notifier *notifier);
void qemu_system_wakeup_request(WakeupReason reason);
void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
void qemu_register_wakeup_notifier(Notifier *notifier);
-void qemu_system_shutdown_request(void);
+void qemu_system_shutdown_request(ShutdownCause reason);
void qemu_system_powerdown_request(void);
void qemu_register_powerdown_notifier(Notifier *notifier);
void qemu_system_debug_request(void);
diff --git a/vl.c b/vl.c
index 51ed60f..bc5c1be 100644
--- a/vl.c
+++ b/vl.c
@@ -1724,7 +1724,7 @@ void qemu_system_guest_panicked(GuestPanicInformation
*info)
if (!no_shutdown) {
qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_POWEROFF,
!!info, info, _abort);
-
[Xen-devel] [PATCH v9 2/5] shutdown: Prepare for use of an enum in reset/shutdown_request
We want to track why a guest was shutdown; in particular, being able
to tell the difference between a guest request (such as ACPI request)
and host request (such as SIGINT) will prove useful to libvirt.
Since all requests eventually end up changing shutdown_requested in
vl.c, the logical change is to make that value track the reason,
rather than its current 0/1 contents.
Since command-line options control whether a reset request is turned
into a shutdown request instead, the same treatment is given to
reset_requested.
This patch adds an internal enum ShutdownCause that describes reasons
that a shutdown can be requested, and changes qemu_system_reset() to
pass the reason through, although for now nothing is actually changed
with regards to what gets reported. The enum could be exported via
QAPI at a later date, if deemed necessary, but for now, there has not
been a request to expose that much detail to end clients.
For the most part, we turn 0 into SHUTDOWN_CAUSE_NONE, and 1 into
SHUTDOWN_CAUSE_HOST_ERROR; the only specific case where we have enough
information right now to use a different value is when we are reacting
to a host signal. It will take a further patch to edit all call-sites
that can trigger a reset or shutdown request to properly pass in any
other reasons; this patch includes TODOs to point such places out.
qemu_system_reset() trades its 'bool report' parameter for a
'ShutdownCause reason', with all non-zero values having the same
effect; this lets us get rid of the weird #defines for VMRESET_*
as synonyms for bools.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
v9: one more stray FIXME
v8: s/FIXME/TODO/, include SHUTDOWN_CAUSE__MAX now rather than later,
tweak comment on GUEST_SHUTDOWN to mention suspend
v7: drop 'bool report' from qemu_system_reset(), reorder enum to put
HOST_ERROR == 1, improve commit message
v6: make ShutdownCause internal-only, add SHUTDOWN_CAUSE_NONE so that
comparison to 0 still works, tweak initial FIXME values
v5: no change
v4: s/ShutdownType/ShutdownCause/, no thanks to mingw header pollution
v3: new patch
---
include/sysemu/sysemu.h | 23 -
vl.c| 53 ++---
hw/i386/xen/xen-hvm.c | 7 +--
migration/colo.c| 2 +-
migration/savevm.c | 2 +-
5 files changed, 58 insertions(+), 29 deletions(-)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 15656b7..52102fd 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -33,8 +33,21 @@ VMChangeStateEntry
*qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
void qemu_del_vm_change_state_handler(VMChangeStateEntry *e);
void vm_state_notify(int running, RunState state);
-#define VMRESET_SILENT false
-#define VMRESET_REPORT true
+/* Enumeration of various causes for shutdown. */
+typedef enum ShutdownCause {
+SHUTDOWN_CAUSE_NONE, /* No shutdown request pending */
+SHUTDOWN_CAUSE_HOST_ERROR,/* An error prevents further use of guest */
+SHUTDOWN_CAUSE_HOST_QMP, /* Reaction to a QMP command, like 'quit' */
+SHUTDOWN_CAUSE_HOST_SIGNAL, /* Reaction to a signal, such as SIGINT */
+SHUTDOWN_CAUSE_HOST_UI, /* Reaction to UI event, like window close */
+SHUTDOWN_CAUSE_GUEST_SHUTDOWN,/* Guest shutdown/suspend request, via
+ ACPI or other hardware-specific means */
+SHUTDOWN_CAUSE_GUEST_RESET, /* Guest reset request, and command line
+ turns that into a shutdown */
+SHUTDOWN_CAUSE_GUEST_PANIC, /* Guest panicked, and command line turns
+ that into a shutdown */
+SHUTDOWN_CAUSE__MAX,
+} ShutdownCause;
void vm_start(void);
int vm_prepare_start(void);
@@ -62,10 +75,10 @@ void qemu_system_debug_request(void);
void qemu_system_vmstop_request(RunState reason);
void qemu_system_vmstop_request_prepare(void);
bool qemu_vmstop_requested(RunState *r);
-int qemu_shutdown_requested_get(void);
-int qemu_reset_requested_get(void);
+ShutdownCause qemu_shutdown_requested_get(void);
+ShutdownCause qemu_reset_requested_get(void);
void qemu_system_killed(int signal, pid_t pid);
-void qemu_system_reset(bool report);
+void qemu_system_reset(ShutdownCause reason);
void qemu_system_guest_panicked(GuestPanicInformation *info);
size_t qemu_target_page_size(void);
diff --git a/vl.c b/vl.c
index 7396748..5c61d8c 100644
--- a/vl.c
+++ b/vl.c
@@ -1597,8 +1597,9 @@ void vm_state_notify(int running, RunState state)
}
}
-static int reset_requested;
-static int shutdown_requested, shutdown_signal;
+static ShutdownCause reset_requested;
+static ShutdownCause shutdown_requested;
+static int shutdown_signal;
static pid_t shutdown_pid;
static int powerdown_requested;
static int debug_requested;
@@ -1612,19 +1613,19 @@ static NotifierList wakeup_notifiers =
NOTIFIER_LIST_INITIALIZER(wakeup_notifiers);
static ui
[Xen-devel] [PATCH v8 4/5] shutdown: Add source information to SHUTDOWN and RESET
Time to wire up all the call sites that request a shutdown or
reset to use the enum added in the previous patch.
It would have been less churn to keep the common case with no
arguments as meaning guest-triggered, and only modified the
host-triggered code paths, via a wrapper function, but then we'd
still have to audit that I didn't miss any host-triggered spots;
changing the signature forces us to double-check that I correctly
categorized all callers.
Since command line options can change whether a guest reset request
causes an actual reset vs. a shutdown, it's easy to also add the
information to reset requests.
Signed-off-by: Eric Blake <ebl...@redhat.com>
Acked-by: David Gibson <da...@gibson.dropbear.id.au> [ppc parts]
Reviewed-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk> [SPARC part]
Reviewed-by: Cornelia Huck <cornelia.h...@de.ibm.com> [s390x parts]
---
v8: rebase later in series
v7: no change
v6: defer event additions to later, add reviews of unchanged portions
v5: drop accidental addition of unrelated files
v4: s/ShutdownType/ShutdownCause/, no thanks to mingw header pollution
v3: retitle again, fix qemu-iotests, use enum rather than raw bool
in all callers
v2: retitle (was "event: Add signal information to SHUTDOWN"),
completely rework to post bool based on whether it is guest-initiated
v1: initial submission, exposing just Unix signals from host
---
include/sysemu/sysemu.h | 4 ++--
vl.c| 18 --
hw/acpi/core.c | 4 ++--
hw/arm/highbank.c | 4 ++--
hw/arm/integratorcp.c | 2 +-
hw/arm/musicpal.c | 2 +-
hw/arm/omap1.c | 10 ++
hw/arm/omap2.c | 2 +-
hw/arm/spitz.c | 2 +-
hw/arm/stellaris.c | 2 +-
hw/arm/tosa.c | 2 +-
hw/i386/pc.c| 2 +-
hw/i386/xen/xen-hvm.c | 2 +-
hw/input/pckbd.c| 4 ++--
hw/ipmi/ipmi.c | 4 ++--
hw/isa/lpc_ich9.c | 2 +-
hw/mips/boston.c| 2 +-
hw/mips/mips_malta.c| 2 +-
hw/mips/mips_r4k.c | 4 ++--
hw/misc/arm_sysctl.c| 8
hw/misc/cbus.c | 2 +-
hw/misc/macio/cuda.c| 4 ++--
hw/misc/slavio_misc.c | 4 ++--
hw/misc/zynq_slcr.c | 2 +-
hw/pci-host/apb.c | 4 ++--
hw/pci-host/bonito.c| 2 +-
hw/pci-host/piix.c | 2 +-
hw/ppc/e500.c | 2 +-
hw/ppc/mpc8544_guts.c | 2 +-
hw/ppc/ppc.c| 2 +-
hw/ppc/ppc405_uc.c | 2 +-
hw/ppc/spapr_hcall.c| 2 +-
hw/ppc/spapr_rtas.c | 4 ++--
hw/s390x/ipl.c | 2 +-
hw/sh4/r2d.c| 2 +-
hw/timer/etraxfs_timer.c| 2 +-
hw/timer/m48t59.c | 4 ++--
hw/timer/milkymist-sysctl.c | 4 ++--
hw/timer/pxa2xx_timer.c | 2 +-
hw/watchdog/watchdog.c | 2 +-
hw/xenpv/xen_domainbuild.c | 2 +-
hw/xtensa/xtfpga.c | 2 +-
kvm-all.c | 6 +++---
os-win32.c | 2 +-
qmp.c | 4 ++--
replay/replay.c | 4 ++--
target/alpha/sys_helper.c | 4 ++--
target/arm/psci.c | 4 ++--
target/i386/excp_helper.c | 2 +-
target/i386/hax-all.c | 6 +++---
target/i386/helper.c| 2 +-
target/i386/kvm.c | 2 +-
target/s390x/helper.c | 2 +-
target/s390x/kvm.c | 4 ++--
target/s390x/misc_helper.c | 4 ++--
target/sparc/int32_helper.c | 2 +-
ui/sdl.c| 2 +-
ui/sdl2.c | 4 ++--
trace-events| 2 +-
ui/cocoa.m | 2 +-
60 files changed, 98 insertions(+), 98 deletions(-)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 52102fd..e540e6f 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -62,13 +62,13 @@ typedef enum WakeupReason {
QEMU_WAKEUP_REASON_OTHER,
} WakeupReason;
-void qemu_system_reset_request(void);
+void qemu_system_reset_request(ShutdownCause reason);
void qemu_system_suspend_request(void);
void qemu_register_suspend_notifier(Notifier *notifier);
void qemu_system_wakeup_request(WakeupReason reason);
void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
void qemu_register_wakeup_notifier(Notifier *notifier);
-void qemu_system_shutdown_request(void);
+void qemu_system_shutdown_request(ShutdownCause reason);
void qemu_system_powerdown_request(void);
void qemu_register_powerdown_notifier(Notifier *notifier);
void qemu_system_debug_request(void);
diff --git a/vl.c b/vl.c
index 4641fdf..808c67b 100644
--- a/vl.c
+++ b/vl.c
@@ -1724,7 +1724,7 @@ void qemu_system_guest_panicked(GuestPanicInformation
*info)
if (!no_shutdown) {
qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_POWEROFF,
!!info, info, _abort);
-
[Xen-devel] [PATCH v8 2/5] shutdown: Prepare for use of an enum in reset/shutdown_request
We want to track why a guest was shutdown; in particular, being able
to tell the difference between a guest request (such as ACPI request)
and host request (such as SIGINT) will prove useful to libvirt.
Since all requests eventually end up changing shutdown_requested in
vl.c, the logical change is to make that value track the reason,
rather than its current 0/1 contents.
Since command-line options control whether a reset request is turned
into a shutdown request instead, the same treatment is given to
reset_requested.
This patch adds an internal enum ShutdownCause that describes reasons
that a shutdown can be requested, and changes qemu_system_reset() to
pass the reason through, although for now nothing is actually changed
with regards to what gets reported. The enum could be exported via
QAPI at a later date, if deemed necessary, but for now, there has not
been a request to expose that much detail to end clients.
For the most part, we turn 0 into SHUTDOWN_CAUSE_NONE, and 1 into
SHUTDOWN_CAUSE_HOST_ERROR; the only specific case where we have enough
information right now to use a different value is when we are reacting
to a host signal. It will take a further patch to edit all call-sites
that can trigger a reset or shutdown request to properly pass in any
other reasons; this patch includes TODOs to point such places out.
qemu_system_reset() trades its 'bool report' parameter for a
'ShutdownCause reason', with all non-zero values having the same
effect; this lets us get rid of the weird #defines for VMRESET_*
as synonyms for bools.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
v8: s/FIXME/TODO/, include SHUTDOWN_CAUSE__MAX now rather than later,
tweak comment on GUEST_SHUTDOWN to mention suspend
v7: drop 'bool report' from qemu_system_reset(), reorder enum to put
HOST_ERROR == 1, improve commit message
v6: make ShutdownCause internal-only, add SHUTDOWN_CAUSE_NONE so that
comparison to 0 still works, tweak initial FIXME values
v5: no change
v4: s/ShutdownType/ShutdownCause/, no thanks to mingw header pollution
v3: new patch
---
include/sysemu/sysemu.h | 23 -
vl.c| 53 ++---
hw/i386/xen/xen-hvm.c | 7 +--
migration/colo.c| 2 +-
migration/savevm.c | 2 +-
5 files changed, 58 insertions(+), 29 deletions(-)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 15656b7..52102fd 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -33,8 +33,21 @@ VMChangeStateEntry
*qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
void qemu_del_vm_change_state_handler(VMChangeStateEntry *e);
void vm_state_notify(int running, RunState state);
-#define VMRESET_SILENT false
-#define VMRESET_REPORT true
+/* Enumeration of various causes for shutdown. */
+typedef enum ShutdownCause {
+SHUTDOWN_CAUSE_NONE, /* No shutdown request pending */
+SHUTDOWN_CAUSE_HOST_ERROR,/* An error prevents further use of guest */
+SHUTDOWN_CAUSE_HOST_QMP, /* Reaction to a QMP command, like 'quit' */
+SHUTDOWN_CAUSE_HOST_SIGNAL, /* Reaction to a signal, such as SIGINT */
+SHUTDOWN_CAUSE_HOST_UI, /* Reaction to UI event, like window close */
+SHUTDOWN_CAUSE_GUEST_SHUTDOWN,/* Guest shutdown/suspend request, via
+ ACPI or other hardware-specific means */
+SHUTDOWN_CAUSE_GUEST_RESET, /* Guest reset request, and command line
+ turns that into a shutdown */
+SHUTDOWN_CAUSE_GUEST_PANIC, /* Guest panicked, and command line turns
+ that into a shutdown */
+SHUTDOWN_CAUSE__MAX,
+} ShutdownCause;
void vm_start(void);
int vm_prepare_start(void);
@@ -62,10 +75,10 @@ void qemu_system_debug_request(void);
void qemu_system_vmstop_request(RunState reason);
void qemu_system_vmstop_request_prepare(void);
bool qemu_vmstop_requested(RunState *r);
-int qemu_shutdown_requested_get(void);
-int qemu_reset_requested_get(void);
+ShutdownCause qemu_shutdown_requested_get(void);
+ShutdownCause qemu_reset_requested_get(void);
void qemu_system_killed(int signal, pid_t pid);
-void qemu_system_reset(bool report);
+void qemu_system_reset(ShutdownCause reason);
void qemu_system_guest_panicked(GuestPanicInformation *info);
size_t qemu_target_page_size(void);
diff --git a/vl.c b/vl.c
index 7396748..2060038 100644
--- a/vl.c
+++ b/vl.c
@@ -1597,8 +1597,9 @@ void vm_state_notify(int running, RunState state)
}
}
-static int reset_requested;
-static int shutdown_requested, shutdown_signal;
+static ShutdownCause reset_requested;
+static ShutdownCause shutdown_requested;
+static int shutdown_signal;
static pid_t shutdown_pid;
static int powerdown_requested;
static int debug_requested;
@@ -1612,19 +1613,19 @@ static NotifierList wakeup_notifiers =
NOTIFIER_LIST_INITIALIZER(wakeup_notifiers);
static uint32_t wakeup_reason_mask
Re: [Xen-devel] [Qemu-devel] [PATCH v2 1/3] xen/9pfs: fix two resource leaks on error paths, discovered by Coverity
On 05/09/2017 02:20 PM, Eric Blake wrote: > On 05/09/2017 02:04 PM, Stefano Stabellini wrote: >> CID: 1374836 >> >> Signed-off-by: Stefano Stabellini <sstabell...@kernel.org> >> CC: anthony.per...@citrix.com >> CC: gr...@kaod.org >> CC: aneesh.ku...@linux.vnet.ibm.com >> --- >> hw/9pfs/xen-9p-backend.c | 2 ++ >> 1 file changed, 2 insertions(+) > > Reviewed-by: Eric Blake <ebl...@redhat.com> By the way, you forgot to send a 0/3 cover letter (at least, there was no In-Reply-To: header in your 1/3 mail). That makes it harder to automate handling of your series; I was going to reply to the series as a whole. While it is inconvenient for human readers, it is even worse for some of the automated patch tooling we have that expects cover letters for any multi-patch series. More patch submission tips at http://wiki.qemu.org/Contribute/SubmitAPatch include how to use 'git config' to automate the creation of a cover letter. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 2/3] Check the return value of fcntl in qemu_set_cloexec
On 05/09/2017 02:04 PM, Stefano Stabellini wrote: > Assert that the return value is not an error. This issue was found by > Coverity. > > CID: 1374831 > > Signed-off-by: Stefano Stabellini <sstabell...@kernel.org> > CC: gr...@kaod.org > CC: pbonz...@redhat.com > CC: Eric Blake <ebl...@redhat.com> > --- > util/oslib-posix.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 3/3] xen: call qemu_set_cloexec instead of fcntl
On 05/09/2017 02:04 PM, Stefano Stabellini wrote: > Use the common utility function, which contains checks on return values, > instead of manually calling fcntl. > > CID: 1374831 > > Signed-off-by: Stefano Stabellini <sstabell...@kernel.org> > CC: anthony.per...@citrix.com > CC: gr...@kaod.org > CC: aneesh.ku...@linux.vnet.ibm.com > CC: Eric Blake <ebl...@redhat.com> > --- > hw/9pfs/xen-9p-backend.c | 2 +- > hw/xen/xen_backend.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH v2 1/3] xen/9pfs: fix two resource leaks on error paths, discovered by Coverity
On 05/09/2017 02:04 PM, Stefano Stabellini wrote: > CID: 1374836 > > Signed-off-by: Stefano Stabellini <sstabell...@kernel.org> > CC: anthony.per...@citrix.com > CC: gr...@kaod.org > CC: aneesh.ku...@linux.vnet.ibm.com > --- > hw/9pfs/xen-9p-backend.c | 2 ++ > 1 file changed, 2 insertions(+) Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH v7 3/5] shutdown: Add source information to SHUTDOWN and RESET
On 05/09/2017 06:56 AM, Markus Armbruster wrote:
> Eric Blake <ebl...@redhat.com> writes:
>
>> Time to wire up all the call sites that request a shutdown or
>> reset to use the enum added in the previous patch.
>>
>> It would have been less churn to keep the common case with no
>> arguments as meaning guest-triggered, and only modified the
>> host-triggered code paths, via a wrapper function, but then we'd
>> still have to audit that I didn't miss any host-triggered spots;
>> changing the signature forces us to double-check that I correctly
>> categorized all callers.
>>
>> Since command line options can change whether a guest reset request
>> causes an actual reset vs. a shutdown, it's easy to also add the
>> information to reset requests.
>>
>> Replay adds a FIXME to preserve the cause across the replay stream,
>> that will be tackled in the next patch.
>>
>> @@ -569,7 +569,7 @@ static void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t
>> val)
>> default:
>> if (sus_typ == ar->pm1.cnt.s4_val) { /* S4 request */
>> qapi_event_send_suspend_disk(_abort);
>> -qemu_system_shutdown_request();
>> +qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
>
> I'm fine with using SHUTDOWN_CAUSE_GUEST_SHUTDOWN for suspend, but have
> you considered SHUTDOWN_CAUSE_GUEST_SUSPEND?
It was easy to do
s/qemu_system_shutdown_request()/qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN)/
for all hw/ files. Harder would be picking a difference between
_SHUTDOWN and a new _SUSPEND. I can do it if hardware owners want the
distinction; but remember that this series will intentionally NOT expose
that distinction to QMP, so I don't know how much it will buy us.
>> void qmp_stop(Error **errp)
>> @@ -105,7 +105,7 @@ void qmp_stop(Error **errp)
>>
>> void qmp_system_reset(Error **errp)
>> {
>> -qemu_system_reset_request();
>> +qemu_system_reset_request(SHUTDOWN_CAUSE_HOST_QMP);
>
> This is the only place where we pass something other than
> SHUTDOWN_CAUSE_GUEST_RESET. We could avoid churn the obvious way, but I
> guess having the churn eases patch review. Okay.
Yes, and that was the comment I made in the commit message about
changing the signature everywhere instead of adding wrappers that make
the common case become the default.
>> +++ b/replay/replay.c
>> @@ -51,7 +51,8 @@ bool replay_next_event_is(int event)
>> switch (replay_state.data_kind) {
>> case EVENT_SHUTDOWN:
>> replay_finish_event();
>> -qemu_system_shutdown_request();
>> +/* FIXME - store actual reason */
>> +qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_ERROR);
>
> The temporary replay breakage is no big deal. Still, can we avoid it by
> extending replay first, using a dummy value like
> SHUTDOWN_CAUSE_HOST_ERROR until the real cause becomes available? Not
> sure it's worth a respin, though.
>
>> break;
>> default:
>> /* clock, time_t, checkpoint and other events */
> [...]
>
> Reviewed-by: Markus Armbruster <arm...@redhat.com>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] Fix issues affecting Xen 9pfs discovered by Coverity
On 05/08/2017 05:00 PM, Stefano Stabellini wrote:
>>> Directly calling fcntl(F_SETFD) without first reading fcntl(F_GETFD) is
>>> (theoretically) incorrect. Better might be using qemu_set_cloexec()
>>> instead of open-coding something.
>>
>> Makes sense but the unchecked return of fcntl, discovered by Coverity,
>> would remain unfixed by calling qemu_set_cloexec here. I don't think I
>> am up for fixing all the call sites of qemu_set_cloexec.
>>
>> I am going to drop this change, and resend this patch was only the other
>> two fixes, fixing 1374836 only.
>
> Unless you would be fine with:
>
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 4d9189e..16894ad 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -182,7 +182,9 @@ void qemu_set_cloexec(int fd)
> {
> int f;
> f = fcntl(fd, F_GETFD);
> -fcntl(fd, F_SETFD, f | FD_CLOEXEC);
> +assert(f != -1);
> +f = fcntl(fd, F_SETFD, f | FD_CLOEXEC);
> +assert(f != -1);
Seems reasonable to me, but I don't know if anyone else would object.
Changes semantics if someone ever calls qemu_set_cloexec(-1) (previously
it would ignore the EBADF failures, now it will abort) - such callers
are arguably broken, so that's okay by me.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v7 2/5] shutdown: Prepare for use of an enum in reset/shutdown_request
We want to track why a guest was shutdown; in particular, being able
to tell the difference between a guest request (such as ACPI request)
and host request (such as SIGINT) will prove useful to libvirt.
Since all requests eventually end up changing shutdown_requested in
vl.c, the logical change is to make that value track the reason,
rather than its current 0/1 contents.
Since command-line options control whether a reset request is turned
into a shutdown request instead, the same treatment is given to
reset_requested.
This patch adds an internal enum ShutdownCause that describes reasons
that a shutdown can be requested, and changes qemu_system_reset() to
pass the reason through, although for now nothing is actually changed
with regards to what gets reported. The enum could be exported via
QAPI at a later date, if deemed necessary, but for now, there has not
been a request to expose that much detail to end clients.
For the most part, we turn 0 into SHUTDOWN_CAUSE_NONE, and 1 into
SHUTDOWN_CAUSE_HOST_ERROR; the only specific case where we have enough
information right now to use a different value is when we are reacting
to a host signal. It will take a further patch to edit all call-sites
that can trigger a reset or shutdown request to properly pass in any
other reasons; this patch includes FIXMEs to point such places out.
qemu_system_reset() trades its 'bool report' parameter for a
'ShutdownCause reason', with all non-zero values having the same
effect; this lets us get rid of the weird #defines for VMRESET_*
as synonyms for bools.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
v7: drop 'bool report' from qemu_system_reset(), reorder enum to put
HOST_ERROR == 1, improve commit message
v6: make ShutdownCause internal-only, add SHUTDOWN_CAUSE_NONE so that
comparison to 0 still works, tweak initial FIXME values
v5: no change
v4: s/ShutdownType/ShutdownCause/, no thanks to mingw header pollution
v3: new patch
---
include/sysemu/sysemu.h | 22 +++-
vl.c| 53 ++---
hw/i386/xen/xen-hvm.c | 7 +--
migration/colo.c| 2 +-
migration/savevm.c | 2 +-
5 files changed, 57 insertions(+), 29 deletions(-)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 15656b7..98b3274 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -33,8 +33,20 @@ VMChangeStateEntry
*qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
void qemu_del_vm_change_state_handler(VMChangeStateEntry *e);
void vm_state_notify(int running, RunState state);
-#define VMRESET_SILENT false
-#define VMRESET_REPORT true
+/* Enumeration of various causes for shutdown. */
+typedef enum ShutdownCause {
+SHUTDOWN_CAUSE_NONE, /* No shutdown request pending */
+SHUTDOWN_CAUSE_HOST_ERROR,/* An error prevents further use of guest */
+SHUTDOWN_CAUSE_HOST_QMP, /* Reaction to a QMP command, like 'quit' */
+SHUTDOWN_CAUSE_HOST_SIGNAL, /* Reaction to a signal, such as SIGINT */
+SHUTDOWN_CAUSE_HOST_UI, /* Reaction to UI event, like window close */
+SHUTDOWN_CAUSE_GUEST_SHUTDOWN,/* Guest requested shutdown, such as via
+ ACPI or other hardware-specific means */
+SHUTDOWN_CAUSE_GUEST_RESET, /* Guest requested reset, and command line
+ turns that into a shutdown */
+SHUTDOWN_CAUSE_GUEST_PANIC, /* Guest panicked, and command line turns
+ that into a shutdown */
+} ShutdownCause;
void vm_start(void);
int vm_prepare_start(void);
@@ -62,10 +74,10 @@ void qemu_system_debug_request(void);
void qemu_system_vmstop_request(RunState reason);
void qemu_system_vmstop_request_prepare(void);
bool qemu_vmstop_requested(RunState *r);
-int qemu_shutdown_requested_get(void);
-int qemu_reset_requested_get(void);
+ShutdownCause qemu_shutdown_requested_get(void);
+ShutdownCause qemu_reset_requested_get(void);
void qemu_system_killed(int signal, pid_t pid);
-void qemu_system_reset(bool report);
+void qemu_system_reset(ShutdownCause reason);
void qemu_system_guest_panicked(GuestPanicInformation *info);
size_t qemu_target_page_size(void);
diff --git a/vl.c b/vl.c
index 7a205e0..8df886e 100644
--- a/vl.c
+++ b/vl.c
@@ -1597,8 +1597,9 @@ void vm_state_notify(int running, RunState state)
}
}
-static int reset_requested;
-static int shutdown_requested, shutdown_signal;
+static ShutdownCause reset_requested;
+static ShutdownCause shutdown_requested;
+static int shutdown_signal;
static pid_t shutdown_pid;
static int powerdown_requested;
static int debug_requested;
@@ -1612,19 +1613,19 @@ static NotifierList wakeup_notifiers =
NOTIFIER_LIST_INITIALIZER(wakeup_notifiers);
static uint32_t wakeup_reason_mask = ~(1 << QEMU_WAKEUP_REASON_NONE);
-int qemu_shutdown_requested_get(void)
+ShutdownCause qemu_shutdown_requested_get(void)
{
[Xen-devel] [PATCH v7 3/5] shutdown: Add source information to SHUTDOWN and RESET
Time to wire up all the call sites that request a shutdown or
reset to use the enum added in the previous patch.
It would have been less churn to keep the common case with no
arguments as meaning guest-triggered, and only modified the
host-triggered code paths, via a wrapper function, but then we'd
still have to audit that I didn't miss any host-triggered spots;
changing the signature forces us to double-check that I correctly
categorized all callers.
Since command line options can change whether a guest reset request
causes an actual reset vs. a shutdown, it's easy to also add the
information to reset requests.
Replay adds a FIXME to preserve the cause across the replay stream,
that will be tackled in the next patch.
Signed-off-by: Eric Blake <ebl...@redhat.com>
Acked-by: David Gibson <da...@gibson.dropbear.id.au> [ppc parts]
Reviewed-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk> [SPARC part]
---
v7: no change
v6: defer event additions to later, add reviews of unchanged portions
v5: drop accidental addition of unrelated files
v4: s/ShutdownType/ShutdownCause/, no thanks to mingw header pollution
v3: retitle again, fix qemu-iotests, use enum rather than raw bool
in all callers
v2: retitle (was "event: Add signal information to SHUTDOWN"),
completely rework to post bool based on whether it is guest-initiated
v1: initial submission, exposing just Unix signals from host
---
include/sysemu/sysemu.h | 4 ++--
vl.c| 17 -
hw/acpi/core.c | 4 ++--
hw/arm/highbank.c | 4 ++--
hw/arm/integratorcp.c | 2 +-
hw/arm/musicpal.c | 2 +-
hw/arm/omap1.c | 10 ++
hw/arm/omap2.c | 2 +-
hw/arm/spitz.c | 2 +-
hw/arm/stellaris.c | 2 +-
hw/arm/tosa.c | 2 +-
hw/i386/pc.c| 2 +-
hw/i386/xen/xen-hvm.c | 2 +-
hw/input/pckbd.c| 4 ++--
hw/ipmi/ipmi.c | 4 ++--
hw/isa/lpc_ich9.c | 2 +-
hw/mips/boston.c| 2 +-
hw/mips/mips_malta.c| 2 +-
hw/mips/mips_r4k.c | 4 ++--
hw/misc/arm_sysctl.c| 8
hw/misc/cbus.c | 2 +-
hw/misc/macio/cuda.c| 4 ++--
hw/misc/slavio_misc.c | 4 ++--
hw/misc/zynq_slcr.c | 2 +-
hw/pci-host/apb.c | 4 ++--
hw/pci-host/bonito.c| 2 +-
hw/pci-host/piix.c | 2 +-
hw/ppc/e500.c | 2 +-
hw/ppc/mpc8544_guts.c | 2 +-
hw/ppc/ppc.c| 2 +-
hw/ppc/ppc405_uc.c | 2 +-
hw/ppc/spapr_hcall.c| 2 +-
hw/ppc/spapr_rtas.c | 4 ++--
hw/s390x/ipl.c | 2 +-
hw/sh4/r2d.c| 2 +-
hw/timer/etraxfs_timer.c| 2 +-
hw/timer/m48t59.c | 4 ++--
hw/timer/milkymist-sysctl.c | 4 ++--
hw/timer/pxa2xx_timer.c | 2 +-
hw/watchdog/watchdog.c | 2 +-
hw/xenpv/xen_domainbuild.c | 2 +-
hw/xtensa/xtfpga.c | 2 +-
kvm-all.c | 6 +++---
os-win32.c | 2 +-
qmp.c | 4 ++--
replay/replay.c | 3 ++-
target/alpha/sys_helper.c | 4 ++--
target/arm/psci.c | 4 ++--
target/i386/excp_helper.c | 2 +-
target/i386/hax-all.c | 6 +++---
target/i386/helper.c| 2 +-
target/i386/kvm.c | 2 +-
target/s390x/helper.c | 2 +-
target/s390x/kvm.c | 4 ++--
target/s390x/misc_helper.c | 4 ++--
target/sparc/int32_helper.c | 2 +-
ui/sdl.c| 2 +-
ui/sdl2.c | 4 ++--
trace-events| 2 +-
ui/cocoa.m | 2 +-
60 files changed, 98 insertions(+), 96 deletions(-)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 98b3274..fe197aa 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -61,13 +61,13 @@ typedef enum WakeupReason {
QEMU_WAKEUP_REASON_OTHER,
} WakeupReason;
-void qemu_system_reset_request(void);
+void qemu_system_reset_request(ShutdownCause reason);
void qemu_system_suspend_request(void);
void qemu_register_suspend_notifier(Notifier *notifier);
void qemu_system_wakeup_request(WakeupReason reason);
void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
void qemu_register_wakeup_notifier(Notifier *notifier);
-void qemu_system_shutdown_request(void);
+void qemu_system_shutdown_request(ShutdownCause reason);
void qemu_system_powerdown_request(void);
void qemu_register_powerdown_notifier(Notifier *notifier);
void qemu_system_debug_request(void);
diff --git a/vl.c b/vl.c
index 8df886e..2d546460 100644
--- a/vl.c
+++ b/vl.c
@@ -1724,7 +1724,7 @@ void qemu_system_guest_panicked(GuestPanicInformation
*info)
if (!no_shutdown) {
qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_POWEROFF,
!!info, info, _abort);
-
Re: [Xen-devel] [Qemu-devel] [PATCH] Fix issues affecting Xen 9pfs discovered by Coverity
On 05/08/2017 03:45 PM, Stefano Stabellini wrote:
> Fix two resource leaks on error paths, discovered by Coverity.
> Check for errors returned by fcntl, also found by Coverity.
>
> CID:1374836
> CID:1374831
>
> @@ -378,7 +380,10 @@ static int xen_9pfs_connect(struct XenDevice *xendev)
> if (xen_9pdev->rings[i].evtchndev == NULL) {
> goto out;
> }
> -fcntl(xenevtchn_fd(xen_9pdev->rings[i].evtchndev), F_SETFD,
> FD_CLOEXEC);
> +if (fcntl(xenevtchn_fd(xen_9pdev->rings[i].evtchndev),
> + F_SETFD, FD_CLOEXEC) == -1) {
> +goto out;
Directly calling fcntl(F_SETFD) without first reading fcntl(F_GETFD) is
(theoretically) incorrect. Better might be using qemu_set_cloexec()
instead of open-coding something.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH v6 2/5] shutdown: Prepare for use of an enum in reset/shutdown_request
On 05/08/2017 01:26 PM, Markus Armbruster wrote:
> Eric Blake <ebl...@redhat.com> writes:
>
>> We want to track why a guest was shutdown; in particular, being able
>> to tell the difference between a guest request (such as ACPI request)
>> and host request (such as SIGINT) will prove useful to libvirt.
>> Since all requests eventually end up changing shutdown_requested in
>> vl.c, the logical change is to make that value track the reason,
>> rather than its current 0/1 contents.
>>
>> Since command-line options control whether a reset request is turned
>> into a shutdown request instead, the same treatment is given to
>> reset_requested.
>>
>> This patch adds an internal enum ShutdownCause that describes reasons
>> that a shutdown can be requested, and changes qemu_system_reset() to
>> pass the reason through, although for now it is not reported. The
>> enum could be exported via QAPI at a later date, if deemed necessary,
>> but for now, there has not been a request to expose that much detail
>> to end clients.
>>
>> For now, we only populate the reason with HOST_ERROR, along with FIXME
>> comments that describe our plans for how to pass an actual correct
>> reason.
>
> In other words, replacing 0 by SHUTDOWN_CAUSE_NONE, and 1 by
> SHUTDOWN_CAUSE_HOST_ERROR. Makes sense.
Maybe I could have ordered HOST_ERROR to actually be 1...
>> +/* Enumeration of various causes for shutdown. */
>> +typedef enum ShutdownCause ShutdownCause;
>> +enum ShutdownCause {
>
> Why define the typedef separately here? What's wrong with
>
> typedef enum ShutdownCause {
> ...
> } ShutdownCause;
>
> ?
That would work too. I don't know if the code base has a strong
preference for one form over the other.
>
>> +SHUTDOWN_CAUSE_NONE, /* No shutdown requested yet */
>
> Comment is fine. Possible alternative: /* No shutdown request pending */
>
>> +SHUTDOWN_CAUSE_HOST_QMP, /* Reaction to a QMP command, like 'quit'
>> */
>> +SHUTDOWN_CAUSE_HOST_SIGNAL, /* Reaction to a signal, such as SIGINT */
>> +SHUTDOWN_CAUSE_HOST_UI, /* Reaction to UI event, like window
>> close */
>> +SHUTDOWN_CAUSE_HOST_ERROR,/* An error prevents further use of guest
>> */
...rather than 4. I don't know that it matters much.
>> -static int qemu_reset_requested(void)
>> +static ShutdownCause qemu_reset_requested(void)
>> {
>> -int r = reset_requested;
>> +ShutdownCause r = reset_requested;
>
> Good opportunity to insert a blank line here.
>
Sure.
>> if (r && replay_checkpoint(CHECKPOINT_RESET_REQUESTED)) {
>> -reset_requested = 0;
>> +reset_requested = SHUTDOWN_CAUSE_NONE;
>> return r;
>> }
>> -return false;
>> +return SHUTDOWN_CAUSE_NONE;
>> }
>>
>> static int qemu_suspend_requested(void)
>> @@ -1686,7 +1687,12 @@ static int qemu_debug_requested(void)
>> return r;
>> }
>>
>> -void qemu_system_reset(bool report)
>> +/*
>> + * Reset the VM. If @report is VMRESET_REPORT, issue an event, using
>> + * the @reason interpreted as ShutdownCause for details. Otherwise,
>> + * @report is VMRESET_SILENT and @reason is ignored.
>> + */
>
> "interpreted as ShutdownCause"? It *is* a ShutdownCause. Leftover?
Oh, yeah. In v5, the parameter was 'int'.
>
>> +void qemu_system_reset(bool report, ShutdownCause reason)
>> {
>> MachineClass *mc;
>>
>> @@ -1700,6 +1706,7 @@ void qemu_system_reset(bool report)
>> qemu_devices_reset();
>> }
>> if (report) {
>> +assert(reason);
>> qapi_event_send_reset(_abort);
>> }
>> cpu_synchronize_all_post_reset();
>
> Looks like we're not using @reason "for details" just yet.
Correct. I can add a FIXME (to be removed in the later patch where it is
used) if that is desired.
>> @@ -1807,7 +1815,7 @@ void qemu_system_killed(int signal, pid_t pid)
>> /* Cannot call qemu_system_shutdown_request directly because
>> * we are in a signal handler.
>> */
>> -shutdown_requested = 1;
>> +shutdown_requested = SHUTDOWN_CAUSE_HOST_SIGNAL;
>
> Should this be SHUTDOWN_CAUSE_HOST_ERROR, to be updated in the next
> patch? Alternatively, tweak this patch's commit message?
This is the one case that we actually do have a strong cause affiliated
with the reason without having to resort to changing function
signatures. Commit message tweak is better.
>> @@ -1846,13 +1855,16 @
Re: [Xen-devel] [PATCH v6 3/5] shutdown: Add source information to SHUTDOWN and RESET
On 05/08/2017 12:26 AM, David Gibson wrote:
> On Fri, May 05, 2017 at 02:38:08PM -0500, Eric Blake wrote:
>> Time to wire up all the call sites that request a shutdown or
>> reset to use the enum added in the previous patch.
>>
>> It would have been less churn to keep the common case with no
>> arguments as meaning guest-triggered, and only modified the
>> host-triggered code paths, via a wrapper function, but then we'd
>> still have to audit that I didn't miss any host-triggered spots;
>> changing the signature forces us to double-check that I correctly
>> categorized all callers.
>>
>> Since command line options can change whether a guest reset request
>> causes an actual reset vs. a shutdown, it's easy to also add the
>> information to reset requests.
>>
>> Replay adds a FIXME to preserve the cause across the replay stream,
>> that will be tackled in the next patch.
>>
>> Signed-off-by: Eric Blake <ebl...@redhat.com>
>> Acked-by: David Gibson <da...@gibson.dropbear.id.au> [ppc parts]
>> Reviewed-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk> [SPARC part]
>
> [snip]
>
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index 9f18f75..2735fe9 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -1166,7 +1166,7 @@ static target_ulong
>> h_client_architecture_support(PowerPCCPU *cpu,
>> spapr_ovec_cleanup(ov5_updates);
>>
>> if (spapr->cas_reboot) {
>> -qemu_system_reset_request();
>> +qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
>
> I'm not 100% sure about this one, since I'm not sure 100% of how the
> different enum values are defined. This one is tripped when feature
> negotiation between firmware and guest can't be satisfied without
> rebooting (next time round the firmware will use some different
> options).
Patch 2/5 introduced the enum. The biggest part of the patch (for now)
is that anything SHUTDOWN_CAUSE_HOST_ will be exposed to the QMP client
as host-triggered, anything SHUTDOWN_CAUSE_GUEST_ will be exposed as
guest-triggered. I basically used SHUTDOWN_CAUSE_GUEST_RESET for any
call to qemu_system_reset_requst() underneath the hw/ tree, because the
hw/ tree is emulating guest behavior and therefore it is presumably a
reset caused by a guest request.
>
> So it's essentially a firmware/hypervisor triggered reset, but one
> that should only ever be tripped during early guest boot. Is
> CAUSE_GUEST_RESET correct for that?
Of course, I'm not an export on SPAPR, so I'll happily change it to
anything else if you think that is more appropriate. But the rule of
thumb I went by is whether this is qemu emulating a bare-metal
reset/shutdown, vs. qemu killing the guest without waiting for guest
instructions to reach some magic
memory/register/ACPI/who-knows-what-else request. While it may happen
only early during guest boot, it is still the guest firmware that is
requesting it, and not qemu causing a unilateral death.
>
> Apart from this, ppc changes
>
> Acked-by: David Gibson <da...@gibson.dropbear.id.au>
Thanks.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 2/5] shutdown: Prepare for use of an enum in reset/shutdown_request
We want to track why a guest was shutdown; in particular, being able
to tell the difference between a guest request (such as ACPI request)
and host request (such as SIGINT) will prove useful to libvirt.
Since all requests eventually end up changing shutdown_requested in
vl.c, the logical change is to make that value track the reason,
rather than its current 0/1 contents.
Since command-line options control whether a reset request is turned
into a shutdown request instead, the same treatment is given to
reset_requested.
This patch adds an internal enum ShutdownCause that describes reasons
that a shutdown can be requested, and changes qemu_system_reset() to
pass the reason through, although for now it is not reported. The
enum could be exported via QAPI at a later date, if deemed necessary,
but for now, there has not been a request to expose that much detail
to end clients.
For now, we only populate the reason with HOST_ERROR, along with FIXME
comments that describe our plans for how to pass an actual correct
reason. The next patches will then actually wire things up to modify
events to report data based on the reason, and to pass the correct enum
value in from various call-sites that can trigger a reset/shutdown (big
enough that it was worth splitting from this patch).
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
v6: make ShutdownCause internal-only, add SHUTDOWN_CAUSE_NONE so that
comparison to 0 still works, tweak initial FIXME values
v5: no change
v4: s/ShutdownType/ShutdownCause/, no thanks to mingw header pollution
v3: new patch
---
include/sysemu/sysemu.h | 22 ++---
vl.c| 51 +++--
hw/i386/xen/xen-hvm.c | 7 +--
migration/colo.c| 2 +-
migration/savevm.c | 2 +-
5 files changed, 58 insertions(+), 26 deletions(-)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 16175f7..e4da9d4 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -36,6 +36,22 @@ void vm_state_notify(int running, RunState state);
#define VMRESET_SILENT false
#define VMRESET_REPORT true
+/* Enumeration of various causes for shutdown. */
+typedef enum ShutdownCause ShutdownCause;
+enum ShutdownCause {
+SHUTDOWN_CAUSE_NONE, /* No shutdown requested yet */
+SHUTDOWN_CAUSE_HOST_QMP, /* Reaction to a QMP command, like 'quit' */
+SHUTDOWN_CAUSE_HOST_SIGNAL, /* Reaction to a signal, such as SIGINT */
+SHUTDOWN_CAUSE_HOST_UI, /* Reaction to UI event, like window close */
+SHUTDOWN_CAUSE_HOST_ERROR,/* An error prevents further use of guest */
+SHUTDOWN_CAUSE_GUEST_SHUTDOWN,/* Guest requested shutdown, such as via
+ ACPI or other hardware-specific means */
+SHUTDOWN_CAUSE_GUEST_RESET, /* Guest requested reset, and command line
+ turns that into a shutdown */
+SHUTDOWN_CAUSE_GUEST_PANIC, /* Guest panicked, and command line turns
+ that into a shutdown */
+};
+
void vm_start(void);
int vm_prepare_start(void);
int vm_stop(RunState state);
@@ -62,10 +78,10 @@ void qemu_system_debug_request(void);
void qemu_system_vmstop_request(RunState reason);
void qemu_system_vmstop_request_prepare(void);
bool qemu_vmstop_requested(RunState *r);
-int qemu_shutdown_requested_get(void);
-int qemu_reset_requested_get(void);
+ShutdownCause qemu_shutdown_requested_get(void);
+ShutdownCause qemu_reset_requested_get(void);
void qemu_system_killed(int signal, pid_t pid);
-void qemu_system_reset(bool report);
+void qemu_system_reset(bool report, ShutdownCause reason);
void qemu_system_guest_panicked(GuestPanicInformation *info);
size_t qemu_target_page_size(void);
diff --git a/vl.c b/vl.c
index f22a3ac..6069fb2 100644
--- a/vl.c
+++ b/vl.c
@@ -1597,8 +1597,9 @@ void vm_state_notify(int running, RunState state)
}
}
-static int reset_requested;
-static int shutdown_requested, shutdown_signal;
+static ShutdownCause reset_requested;
+static ShutdownCause shutdown_requested;
+static int shutdown_signal;
static pid_t shutdown_pid;
static int powerdown_requested;
static int debug_requested;
@@ -1612,19 +1613,19 @@ static NotifierList wakeup_notifiers =
NOTIFIER_LIST_INITIALIZER(wakeup_notifiers);
static uint32_t wakeup_reason_mask = ~(1 << QEMU_WAKEUP_REASON_NONE);
-int qemu_shutdown_requested_get(void)
+ShutdownCause qemu_shutdown_requested_get(void)
{
return shutdown_requested;
}
-int qemu_reset_requested_get(void)
+ShutdownCause qemu_reset_requested_get(void)
{
return reset_requested;
}
static int qemu_shutdown_requested(void)
{
-return atomic_xchg(_requested, 0);
+return atomic_xchg(_requested, SHUTDOWN_CAUSE_NONE);
}
static void qemu_kill_report(void)
@@ -1647,14 +1648,14 @@ static void qemu_kill_report(void)
}
}
-static int qemu_reset_requested(void)
+static S
[Xen-devel] [PATCH v6 3/5] shutdown: Add source information to SHUTDOWN and RESET
Time to wire up all the call sites that request a shutdown or
reset to use the enum added in the previous patch.
It would have been less churn to keep the common case with no
arguments as meaning guest-triggered, and only modified the
host-triggered code paths, via a wrapper function, but then we'd
still have to audit that I didn't miss any host-triggered spots;
changing the signature forces us to double-check that I correctly
categorized all callers.
Since command line options can change whether a guest reset request
causes an actual reset vs. a shutdown, it's easy to also add the
information to reset requests.
Replay adds a FIXME to preserve the cause across the replay stream,
that will be tackled in the next patch.
Signed-off-by: Eric Blake <ebl...@redhat.com>
Acked-by: David Gibson <da...@gibson.dropbear.id.au> [ppc parts]
Reviewed-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk> [SPARC part]
---
v6: defer event additions to later, add reviews of unchanged portions
v5: drop accidental addition of unrelated files
v4: s/ShutdownType/ShutdownCause/, no thanks to mingw header pollution
v3: retitle again, fix qemu-iotests, use enum rather than raw bool
in all callers
v2: retitle (was "event: Add signal information to SHUTDOWN"),
completely rework to post bool based on whether it is guest-initiated
v1: initial submission, exposing just Unix signals from host
---
include/sysemu/sysemu.h | 4 ++--
vl.c| 17 -
hw/acpi/core.c | 4 ++--
hw/arm/highbank.c | 4 ++--
hw/arm/integratorcp.c | 2 +-
hw/arm/musicpal.c | 2 +-
hw/arm/omap1.c | 10 ++
hw/arm/omap2.c | 2 +-
hw/arm/spitz.c | 2 +-
hw/arm/stellaris.c | 2 +-
hw/arm/tosa.c | 2 +-
hw/i386/pc.c| 2 +-
hw/i386/xen/xen-hvm.c | 2 +-
hw/input/pckbd.c| 4 ++--
hw/ipmi/ipmi.c | 4 ++--
hw/isa/lpc_ich9.c | 2 +-
hw/mips/boston.c| 2 +-
hw/mips/mips_malta.c| 2 +-
hw/mips/mips_r4k.c | 4 ++--
hw/misc/arm_sysctl.c| 8
hw/misc/cbus.c | 2 +-
hw/misc/macio/cuda.c| 4 ++--
hw/misc/slavio_misc.c | 4 ++--
hw/misc/zynq_slcr.c | 2 +-
hw/pci-host/apb.c | 4 ++--
hw/pci-host/bonito.c| 2 +-
hw/pci-host/piix.c | 2 +-
hw/ppc/e500.c | 2 +-
hw/ppc/mpc8544_guts.c | 2 +-
hw/ppc/ppc.c| 2 +-
hw/ppc/ppc405_uc.c | 2 +-
hw/ppc/spapr_hcall.c| 2 +-
hw/ppc/spapr_rtas.c | 4 ++--
hw/s390x/ipl.c | 2 +-
hw/sh4/r2d.c| 2 +-
hw/timer/etraxfs_timer.c| 2 +-
hw/timer/m48t59.c | 4 ++--
hw/timer/milkymist-sysctl.c | 4 ++--
hw/timer/pxa2xx_timer.c | 2 +-
hw/watchdog/watchdog.c | 2 +-
hw/xenpv/xen_domainbuild.c | 2 +-
hw/xtensa/xtfpga.c | 2 +-
kvm-all.c | 6 +++---
os-win32.c | 2 +-
qmp.c | 4 ++--
replay/replay.c | 3 ++-
target/alpha/sys_helper.c | 4 ++--
target/arm/psci.c | 4 ++--
target/i386/excp_helper.c | 2 +-
target/i386/hax-all.c | 6 +++---
target/i386/helper.c| 2 +-
target/i386/kvm.c | 2 +-
target/s390x/helper.c | 2 +-
target/s390x/kvm.c | 4 ++--
target/s390x/misc_helper.c | 4 ++--
target/sparc/int32_helper.c | 2 +-
ui/sdl.c| 2 +-
ui/sdl2.c | 4 ++--
trace-events| 2 +-
ui/cocoa.m | 2 +-
60 files changed, 98 insertions(+), 96 deletions(-)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index e4da9d4..89d0e3e 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -65,13 +65,13 @@ typedef enum WakeupReason {
QEMU_WAKEUP_REASON_OTHER,
} WakeupReason;
-void qemu_system_reset_request(void);
+void qemu_system_reset_request(ShutdownCause reason);
void qemu_system_suspend_request(void);
void qemu_register_suspend_notifier(Notifier *notifier);
void qemu_system_wakeup_request(WakeupReason reason);
void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
void qemu_register_wakeup_notifier(Notifier *notifier);
-void qemu_system_shutdown_request(void);
+void qemu_system_shutdown_request(ShutdownCause reason);
void qemu_system_powerdown_request(void);
void qemu_register_powerdown_notifier(Notifier *notifier);
void qemu_system_debug_request(void);
diff --git a/vl.c b/vl.c
index 6069fb2..9579d5f 100644
--- a/vl.c
+++ b/vl.c
@@ -1725,7 +1725,7 @@ void qemu_system_guest_panicked(GuestPanicInformation
*info)
if (!no_shutdown) {
qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_POWEROFF,
!!info, info, _abort);
-
Re: [Xen-devel] [Qemu-devel] [PATCH v5 2/4] shutdown: Prepare for use of an enum in reset/shutdown_request
On 04/28/2017 09:42 AM, Markus Armbruster wrote:
> Eric Blake <ebl...@redhat.com> writes:
>
>> We want to track why a guest was shutdown; in particular, being able
>> to tell the difference between a guest request (such as ACPI request)
>> and host request (such as SIGINT) will prove useful to libvirt.
>> Since all requests eventually end up changing shutdown_requested in
>> vl.c, the logical change is to make that value track the reason,
>> rather than its current 0/1 contents.
>>
>> Since command-line options control whether a reset request is turned
>> into a shutdown request instead, the same treatment is given to
>> reset_requested.
>>
>> This patch adds a QAPI enum ShutdownCause that describes reasons
>> that a shutdown can be requested, and changes qemu_system_reset() to
>> pass the reason through, although for now it is not reported. The
>> next patch will actually wire things up to modify events to report
>> data based on the reason, and to pass the correct enum value in from
>> various call-sites that can trigger a reset/shutdown. Since QAPI
>> generates enums starting at 0, it's easier if we use a different
>> number as our sentinel that no request has happened yet. Most of
>> the changes are in vl.c, but xen was using things externally.
>>
>> -static int reset_requested;
>> -static int shutdown_requested, shutdown_signal;
>> +static int reset_requested = -1;
>> +static int shutdown_requested = -1, shutdown_signal;
>
> Peeking ahead, I see that shutdown_requested and reset_requested take
> ShutdownCause values and -1. The latter means "no shutdown requested".
> What about adding 'none' to ShutdownCause, with value 0, und use that
> instead of literal -1? Would avoid the unusual "negative means false,
> non-negative means true".
Works nicely if the enum is internal-use only. Gets a bit more awkward
if the enum is exposed to the end-user.
The fact that I let QAPI generate the enum in patch 3 is evidence that
I'm leaning towards exposing it to the end user (patch 4); if we want to
keep it internal-only, a better place for the enum might be in sysemu.h
(where we also have the weird '#define VMRESET_SILENT false' '#define
VMRESET_REPORT true' to name a boolean parameter).
>
> PATCH 4 exposes ShutdownCause in event SHUTDOWN, and 'none' must not
> occur there. However, if we ever add a query-shutdown to go with this
> event, we will need 'none' there.
So, query-shutdown would basically be: what is the last-reported
shutdown event (normally none, when the guest is still running; but if,
like libvirt, you start qemu -no-shutdown, it can then be the cause of
why we are in a shutdown/stopped state while waiting for final cleanup)?
How important/likely is such an event? (Hmm, from libvirt's
perspective, events are usually reliable, but can be lost; if we can
restart libvirtd and reconnect to a qemu process that is hanging on to
life only because no one has cleaned it up yet, query-shutdown does seem
like a useful thing for libvirt to have at the time it reconnects to
that qemu process).
We could always include 'none' in the QAPI enum, then document in
'SHUTDOWN' and 'RESET' events that the cause will never be 'none'. Doc
hacks like that feel a little unclean, but not so horrible as to be
unforgivable.
>
> I'd be tempted to reshuffle declarations here, because shutdown_signal's
> int is a different one than reset_requested's and shutdown_requested,
> and the latter two's "negative means false, non-negative means true" is
> unusual enough to justify a comment.
...
>
> Hmm. In case we stick to literal -1: consider splitting this patch into
> a part that changes @shutdown_requested from zero/non-zero to
> negative/non-negative, and a part that uses ShutdownCause for the
> non-negative values.
You're definitely right that if the enum doesn't have a nice none=0
state, then reshuffling to the magic -1 as no request is awkward enough
to be done alone.
But part of the answer is also dependent on whether we want PATCH 4 or
not (or, as you brought up, the possibility of a query-shutdown command
with even more persistent storage of the last-reported event).
>> @@ -1650,11 +1650,11 @@ static void qemu_kill_report(void)
>> static int qemu_reset_requested(void)
>> {
>> int r = reset_requested;
>> -if (r && replay_checkpoint(CHECKPOINT_RESET_REQUESTED)) {
>> -reset_requested = 0;
>> +if (r >= 0 && replay_checkpoint(CHECKPOINT_RESET_REQUESTED)) {
>> +reset_requested = -1;
>> return r;
>> }
>> -return false;
>> +return -1;
>
> "return false" in a function returning int smells, g
Re: [Xen-devel] [PATCH v5 2/4] shutdown: Prepare for use of an enum in reset/shutdown_request
On 04/28/2017 11:09 AM, Dr. David Alan Gilbert wrote: >>> At a higher level, using your tags, I'm not sure where a reset triggered >>> by a fault detected by the hypervisor lives - e.g. an x86 triple fault >>> where the guest screws up so badly that it just gets reset. Is >>> that a guest-reset or a guest-panic or what - neither case >>> was actually asked for by the guest itself. >> >> Wouldn't that be host-error (qemu detected an error that prevents >> further execution of the guest without a reset - and a triple fault >> seems to fall into the category of the guest getting itself wedged >> rather than actually trying to reset)? Except patch 3 only used >> SHUTDOWN_TYPE_HOST_ERROR in the xen portion of the patch. >> >> So if any x86 expert has an opinion on where triple-fault handling is >> emulated, and what category should be used there, I'm welcome to >> tweaking this series. > > It's pretty much on the border anyway, I don't think it matters too > much; it sounds perfectly reasonable. Actually, reading https://blogs.msdn.microsoft.com/larryosterman/2005/02/08/faster-syscall-trap-redux/ makes it sound like the triple-fault = reset is exploited by existing OS (dating back to days of targetting 286 machines), so it is bare-metal behavior that we have to faithfully emulate as a guest-triggered reset, and not something where the guest has wedged itself to the point where qemu can no longer execute the guest. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5 2/4] shutdown: Prepare for use of an enum in reset/shutdown_request
On 04/28/2017 10:27 AM, Dr. David Alan Gilbert wrote: >>>> +# Enumeration of various causes for shutdown. >>>> +# >>>> +# @host-qmp: Reaction to a QMP command, such as 'quit' >>>> +# @host-signal: Reaction to a signal, such as SIGINT >>>> +# @host-ui: Reaction to a UI event, such as closing the window >>>> +# @host-replay: The host is replaying an earlier shutdown event >>>> +# @host-error: Qemu encountered an error that prevents further use of the >>>> guest >>>> +# @guest-shutdown: The guest requested a shutdown, such as via ACPI or >>>> +# other hardware-specific action >>>> +# @guest-reset: The guest requested a reset, and the command line >>>> +# response to a reset is to instead trigger a shutdown >>>> +# @guest-panic: The guest panicked, and the command line response to >>>> +# a panic is to trigger a shutdown >>> > At a higher level, using your tags, I'm not sure where a reset triggered > by a fault detected by the hypervisor lives - e.g. an x86 triple fault > where the guest screws up so badly that it just gets reset. Is > that a guest-reset or a guest-panic or what - neither case > was actually asked for by the guest itself. Wouldn't that be host-error (qemu detected an error that prevents further execution of the guest without a reset - and a triple fault seems to fall into the category of the guest getting itself wedged rather than actually trying to reset)? Except patch 3 only used SHUTDOWN_TYPE_HOST_ERROR in the xen portion of the patch. So if any x86 expert has an opinion on where triple-fault handling is emulated, and what category should be used there, I'm welcome to tweaking this series. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5 2/4] shutdown: Prepare for use of an enum in reset/shutdown_request
On 04/28/2017 03:08 AM, Dr. David Alan Gilbert wrote: > * Eric Blake (ebl...@redhat.com) wrote: >> We want to track why a guest was shutdown; in particular, being able >> to tell the difference between a guest request (such as ACPI request) >> and host request (such as SIGINT) will prove useful to libvirt. >> Since all requests eventually end up changing shutdown_requested in >> vl.c, the logical change is to make that value track the reason, >> rather than its current 0/1 contents. >> >> ## >> +# @ShutdownCause: >> +# >> +# Enumeration of various causes for shutdown. >> +# >> +# @host-qmp: Reaction to a QMP command, such as 'quit' >> +# @host-signal: Reaction to a signal, such as SIGINT >> +# @host-ui: Reaction to a UI event, such as closing the window >> +# @host-replay: The host is replaying an earlier shutdown event >> +# @host-error: Qemu encountered an error that prevents further use of the >> guest >> +# @guest-shutdown: The guest requested a shutdown, such as via ACPI or >> +# other hardware-specific action >> +# @guest-reset: The guest requested a reset, and the command line >> +# response to a reset is to instead trigger a shutdown >> +# @guest-panic: The guest panicked, and the command line response to >> +# a panic is to trigger a shutdown > > It's a little coarse grained; is there anyway to pass platform specific > information > for debug? I ask because I spent a while debugging a few bugs with unexpected > resets and had to figure out which of x86's many reset causes triggered it. I'm open to any followup patches that add further enum values and adjusts the various callers (patch 3 shows how MANY callers use qemu_system_shutdown_request). But I don't think it's necessarily in scope for this series - remember, my goal here was merely to distinguish between host- and guest-triggered resets (which libvirt and higher management tasks want to know), rather than which of multiple reset paths was taken (I agree that it is useful during a qemu debug session - but that's a different audience). I also don't consider myself an expert in the many ways that x86 can reset - it was easy to blindly rewrite qemu_system_shutdown_request() into qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN) based solely on directory, but it would be harder to distinguish which of the multiple files should have which finer-grained cause. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5 2/4] shutdown: Prepare for use of an enum in reset/shutdown_request
We want to track why a guest was shutdown; in particular, being able
to tell the difference between a guest request (such as ACPI request)
and host request (such as SIGINT) will prove useful to libvirt.
Since all requests eventually end up changing shutdown_requested in
vl.c, the logical change is to make that value track the reason,
rather than its current 0/1 contents.
Since command-line options control whether a reset request is turned
into a shutdown request instead, the same treatment is given to
reset_requested.
This patch adds a QAPI enum ShutdownCause that describes reasons
that a shutdown can be requested, and changes qemu_system_reset() to
pass the reason through, although for now it is not reported. The
next patch will actually wire things up to modify events to report
data based on the reason, and to pass the correct enum value in from
various call-sites that can trigger a reset/shutdown. Since QAPI
generates enums starting at 0, it's easier if we use a different
number as our sentinel that no request has happened yet. Most of
the changes are in vl.c, but xen was using things externally.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
v4: s/ShutdownType/ShutdownCause/, no thanks to mingw header pollution
v3: new patch
---
qapi-schema.json| 23 +++
include/sysemu/sysemu.h | 2 +-
vl.c| 44
hw/i386/xen/xen-hvm.c | 9 ++---
migration/colo.c| 2 +-
migration/savevm.c | 2 +-
6 files changed, 60 insertions(+), 22 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 01b087f..a4ebdd1 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2304,6 +2304,29 @@
{ 'command': 'system_powerdown' }
##
+# @ShutdownCause:
+#
+# Enumeration of various causes for shutdown.
+#
+# @host-qmp: Reaction to a QMP command, such as 'quit'
+# @host-signal: Reaction to a signal, such as SIGINT
+# @host-ui: Reaction to a UI event, such as closing the window
+# @host-replay: The host is replaying an earlier shutdown event
+# @host-error: Qemu encountered an error that prevents further use of the guest
+# @guest-shutdown: The guest requested a shutdown, such as via ACPI or
+# other hardware-specific action
+# @guest-reset: The guest requested a reset, and the command line
+# response to a reset is to instead trigger a shutdown
+# @guest-panic: The guest panicked, and the command line response to
+# a panic is to trigger a shutdown
+#
+# Since: 2.10
+##
+{ 'enum': 'ShutdownCause',
+ 'data': [ 'host-qmp', 'host-signal', 'host-ui', 'host-replay', 'host-error',
+'guest-shutdown', 'guest-reset', 'guest-panic' ] }
+
+##
# @cpu:
#
# This command is a nop that is only provided for the purposes of
compatibility.
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 16175f7..00a907f 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -65,7 +65,7 @@ bool qemu_vmstop_requested(RunState *r);
int qemu_shutdown_requested_get(void);
int qemu_reset_requested_get(void);
void qemu_system_killed(int signal, pid_t pid);
-void qemu_system_reset(bool report);
+void qemu_system_reset(bool report, int reason);
void qemu_system_guest_panicked(GuestPanicInformation *info);
size_t qemu_target_page_size(void);
diff --git a/vl.c b/vl.c
index 879786a..2b95b7f 100644
--- a/vl.c
+++ b/vl.c
@@ -1597,8 +1597,8 @@ void vm_state_notify(int running, RunState state)
}
}
-static int reset_requested;
-static int shutdown_requested, shutdown_signal;
+static int reset_requested = -1;
+static int shutdown_requested = -1, shutdown_signal;
static pid_t shutdown_pid;
static int powerdown_requested;
static int debug_requested;
@@ -1624,7 +1624,7 @@ int qemu_reset_requested_get(void)
static int qemu_shutdown_requested(void)
{
-return atomic_xchg(_requested, 0);
+return atomic_xchg(_requested, -1);
}
static void qemu_kill_report(void)
@@ -1650,11 +1650,11 @@ static void qemu_kill_report(void)
static int qemu_reset_requested(void)
{
int r = reset_requested;
-if (r && replay_checkpoint(CHECKPOINT_RESET_REQUESTED)) {
-reset_requested = 0;
+if (r >= 0 && replay_checkpoint(CHECKPOINT_RESET_REQUESTED)) {
+reset_requested = -1;
return r;
}
-return false;
+return -1;
}
static int qemu_suspend_requested(void)
@@ -1686,7 +1686,12 @@ static int qemu_debug_requested(void)
return r;
}
-void qemu_system_reset(bool report)
+/*
+ * Reset the VM. If @report is VMRESET_REPORT, issue an event, using
+ * the @reason interpreted as ShutdownType for details. Otherwise,
+ * @report is VMRESET_SILENT and @reason is ignored.
+ */
+void qemu_system_reset(bool report, int reason)
{
MachineClass *mc;
@@ -1700,6 +1705,7 @@ void qemu_system_reset(bool report)
qemu_devices_reset();
}
if (report) {
+
[Xen-devel] [PATCH v5 3/4] shutdown: Add source information to SHUTDOWN and RESET
Libvirt would like to be able to distinguish between a SHUTDOWN
event triggered solely by guest request and one triggered by a
SIGTERM or other action on the host. While qemu_kill_report() is
already able to tell whether a shutdown was triggered by a host
signal (but NOT by a host UI event, such as clicking the X on
the window), that information was then lost after being printed
to stderr. The previous patch prepped things to use an enum
internally; now it's time to wire it up through all callers, and
to extend the SHUTDOWN and RESET events to report the details.
Enhance the shutdown request path to take a parameter of which
way it is being triggered, and update ALL callers. It would have
been less churn to keep the common case with no arguments as
meaning guest-triggered, and only modified the host-triggered
code paths, via a wrapper function, but then we'd still have to
audit that I didn't miss any host-triggered spots; changing the
signature forces us to double-check that I correctly categorized
all callers.
Since command line options can change whether a guest reset request
causes an actual reset vs. a shutdown, it's easy to also add the
information to the RESET event, even though libvirt has not yet
expressed a need to know that.
For the moment, we keep the enum ShutdownCause for internal use
only, and merely expose a single boolean of 'guest':true|false
to the QMP client; this is because we don't yet have evidence that
the further distinctions will be useful, or whether the addition
of new enum members would cause problems to clients coded to an
older version of the enum.
Update expected iotest outputs to match the new data.
Here is output from 'virsh qemu-monitor-event --loop' with the
patch installed:
event SHUTDOWN at 1492639680.731251 for domain fedora_13: {"guest":true}
event STOP at 1492639680.732116 for domain fedora_13:
event SHUTDOWN at 1492639680.732830 for domain fedora_13: {"guest":false}
Note that libvirt runs qemu with -no-quit: the first SHUTDOWN event
was triggered by an action I took directly in the guest (shutdown -h),
at which point qemu stops the vcpus and waits for libvirt to do any
final cleanups; the second SHUTDOWN event is the result of libvirt
sending SIGTERM now that it has completed cleanup.
The replay driver needs a followup patch if we want to be able to
faithfully replay the difference between a host- and guest-initiated
shutdown (for now, the replayed event is always attributed to host).
See also https://bugzilla.redhat.com/1384007
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
v5: drop accidental addition of unrelated files
v4: s/ShutdownType/ShutdownCause/, no thanks to mingw header pollution
v3: retitle again, fix qemu-iotests, use enum rather than raw bool
in all callers
v2: retitle (was "event: Add signal information to SHUTDOWN"),
completely rework to post bool based on whether it is guest-initiated
v1: initial submission, exposing just Unix signals from host
---
qapi/event.json | 17 +
include/sysemu/sysemu.h | 4 ++--
vl.c| 22 +++---
hw/acpi/core.c | 4 ++--
hw/arm/highbank.c | 4 ++--
hw/arm/integratorcp.c | 2 +-
hw/arm/musicpal.c | 2 +-
hw/arm/omap1.c | 10 ++
hw/arm/omap2.c | 2 +-
hw/arm/spitz.c | 2 +-
hw/arm/stellaris.c | 2 +-
hw/arm/tosa.c | 2 +-
hw/i386/pc.c| 2 +-
hw/i386/xen/xen-hvm.c | 2 +-
hw/input/pckbd.c| 4 ++--
hw/ipmi/ipmi.c | 4 ++--
hw/isa/lpc_ich9.c | 2 +-
hw/mips/boston.c| 2 +-
hw/mips/mips_malta.c| 2 +-
hw/mips/mips_r4k.c | 4 ++--
hw/misc/arm_sysctl.c| 8
hw/misc/cbus.c | 2 +-
hw/misc/macio/cuda.c| 4 ++--
hw/misc/slavio_misc.c | 4 ++--
hw/misc/zynq_slcr.c | 2 +-
hw/pci-host/apb.c | 4 ++--
hw/pci-host/bonito.c| 2 +-
hw/pci-host/piix.c | 2 +-
hw/ppc/e500.c | 2 +-
hw/ppc/mpc8544_guts.c | 2 +-
hw/ppc/ppc.c| 2 +-
hw/ppc/ppc405_uc.c | 2 +-
hw/ppc/spapr_hcall.c| 2 +-
hw/ppc/spapr_rtas.c | 4 ++--
hw/s390x/ipl.c | 2 +-
hw/sh4/r2d.c| 2 +-
hw/timer/etraxfs_timer.c| 2 +-
hw/timer/m48t59.c | 4 ++--
hw/timer/milkymist-sysctl.c | 4 ++--
hw/timer/pxa2xx_timer.c | 2 +-
hw/watchdog/watchdog.c | 2 +-
hw/xenpv/xen_domainbuild.c | 2 +-
hw/xtensa/xtfpga.c | 2 +-
kvm-all.c | 6 +++---
os-win32.c | 2 +-
qmp.c | 4 ++--
replay/replay.c | 5 -
target/alpha/sys_helper.c | 4 ++--
target/arm/psci.c | 4 ++--
target/i386/excp_helper.c | 2 +-
target/i386/hax-all.c | 6 +++-
[Xen-devel] [PATCH v5 06/10] qobject: Use simpler QDict/QList scalar insertion macros
We now have macros in place to make it less verbose to add a scalar
to QDict and QList, so use them. To make this patch smaller to
review, a couple of subdirectories were done in earlier patches.
Patch created mechanically via:
spatch --sp-file scripts/coccinelle/qobject.cocci \
--macro-file scripts/cocci-macro-file.h --dir . --in-place
then touched up manually to fix a couple of '?:' back to original
spacing, as well as avoiding a long line in monitor.c.
Signed-off-by: Eric Blake <ebl...@redhat.com>
Reviewed-by: Markus Armbruster <arm...@redhat.com>
---
v5: rebase to master (Coccinelle found a couple new spots), squash 3
patches into 1, adjust R-b to only list Markus (while there were other
reviews on the pre-squashed patches, Markus was the only one on all 3)
v4: no change
v3: new patch
---
block.c | 45 +---
block/blkdebug.c| 6 +-
block/blkverify.c | 6 +-
block/curl.c| 2 +-
block/file-posix.c | 8 +--
block/file-win32.c | 4 +-
block/nbd.c | 41 ++-
block/nfs.c | 43 +---
block/null.c| 2 +-
block/qcow2.c | 4 +-
block/quorum.c | 8 +--
block/rbd.c | 16 ++---
block/snapshot.c| 2 +-
block/ssh.c | 16 ++---
block/vvfat.c | 10 +--
block/vxhs.c| 6 +-
blockdev.c | 30
hw/block/xen_disk.c | 2 +-
hw/usb/xen-usb.c| 12 ++--
monitor.c | 23 +++
qapi/qmp-event.c| 2 +-
qemu-img.c | 6 +-
qemu-io.c | 2 +-
qemu-nbd.c | 2 +-
qobject/qdict.c | 2 +-
target/s390x/cpu_models.c | 4 +-
tests/check-qdict.c | 134 ++--
tests/check-qlist.c | 4 +-
tests/device-introspect-test.c | 4 +-
tests/test-qemu-opts.c | 4 +-
tests/test-qmp-commands.c | 24 +++
tests/test-qmp-event.c | 30
tests/test-qobject-output-visitor.c | 6 +-
util/qemu-option.c | 2 +-
34 files changed, 244 insertions(+), 268 deletions(-)
diff --git a/block.c b/block.c
index 86c0a61..5b70f58 100644
--- a/block.c
+++ b/block.c
@@ -974,16 +974,14 @@ static void update_flags_from_options(int *flags,
QemuOpts *opts)
static void update_options_from_flags(QDict *options, int flags)
{
if (!qdict_haskey(options, BDRV_OPT_CACHE_DIRECT)) {
-qdict_put(options, BDRV_OPT_CACHE_DIRECT,
- qbool_from_bool(flags & BDRV_O_NOCACHE));
+qdict_put_bool(options, BDRV_OPT_CACHE_DIRECT, flags & BDRV_O_NOCACHE);
}
if (!qdict_haskey(options, BDRV_OPT_CACHE_NO_FLUSH)) {
-qdict_put(options, BDRV_OPT_CACHE_NO_FLUSH,
- qbool_from_bool(flags & BDRV_O_NO_FLUSH));
+qdict_put_bool(options, BDRV_OPT_CACHE_NO_FLUSH,
+ flags & BDRV_O_NO_FLUSH);
}
if (!qdict_haskey(options, BDRV_OPT_READ_ONLY)) {
-qdict_put(options, BDRV_OPT_READ_ONLY,
- qbool_from_bool(!(flags & BDRV_O_RDWR)));
+qdict_put_bool(options, BDRV_OPT_READ_ONLY, !(flags & BDRV_O_RDWR));
}
}
@@ -1399,7 +1397,7 @@ static int bdrv_fill_options(QDict **options, const char
*filename,
/* Fetch the file name from the options QDict if necessary */
if (protocol && filename) {
if (!qdict_haskey(*options, "filename")) {
-qdict_put(*options, "filename", qstring_from_str(filename));
+qdict_put_str(*options, "filename", filename);
parse_filename = true;
} else {
error_setg(errp, "Can't specify 'file' and 'filename' options at "
@@ -1420,7 +1418,7 @@ static int bdrv_fill_options(QDict **options, const char
*filename,
}
drvname = drv->format_name;
-qdict_put(*options, "driver", qstring_from_str(drvname));
+qdict_put_str(*options, "driver", drvname);
} else {
error_setg(errp, "Must specify either driver or file");
return -EINVAL;
@@ -2075,7 +2073,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict
*parent_options,
}
if (bs->backing_format[0] != '\0' && !qdict_haskey(options, "driver")) {
-qdict_put(options, "driver", qstring_from_str(bs->backing_format));
+qdict_put_str(options, "driver", bs->backing_format);
}
backing
[Xen-devel] [PATCH v4 2/4] shutdown: Prepare for use of an enum in reset/shutdown_request
We want to track why a guest was shutdown; in particular, being able
to tell the difference between a guest request (such as ACPI request)
and host request (such as SIGINT) will prove useful to libvirt.
Since all requests eventually end up changing shutdown_requested in
vl.c, the logical change is to make that value track the reason,
rather than its current 0/1 contents.
Since command-line options control whether a reset request is turned
into a shutdown request instead, the same treatment is given to
reset_requested.
This patch adds a QAPI enum ShutdownCause that describes reasons
that a shutdown can be requested, and changes qemu_system_reset() to
pass the reason through, although for now it is not reported. The
next patch will actually wire things up to modify events to report
data based on the reason, and to pass the correct enum value in from
various call-sites that can trigger a reset/shutdown. Since QAPI
generates enums starting at 0, it's easier if we use a different
number as our sentinel that no request has happened yet. Most of
the changes are in vl.c, but xen was using things externally.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
v4: s/ShutdownType/ShutdownCause/, no thanks to mingw header pollution
v3: new patch
---
qapi-schema.json| 23 +++
include/sysemu/sysemu.h | 2 +-
vl.c| 44
hw/i386/xen/xen-hvm.c | 9 ++---
migration/colo.c| 2 +-
migration/savevm.c | 2 +-
6 files changed, 60 insertions(+), 22 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 01b087f..a4ebdd1 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2304,6 +2304,29 @@
{ 'command': 'system_powerdown' }
##
+# @ShutdownCause:
+#
+# Enumeration of various causes for shutdown.
+#
+# @host-qmp: Reaction to a QMP command, such as 'quit'
+# @host-signal: Reaction to a signal, such as SIGINT
+# @host-ui: Reaction to a UI event, such as closing the window
+# @host-replay: The host is replaying an earlier shutdown event
+# @host-error: Qemu encountered an error that prevents further use of the guest
+# @guest-shutdown: The guest requested a shutdown, such as via ACPI or
+# other hardware-specific action
+# @guest-reset: The guest requested a reset, and the command line
+# response to a reset is to instead trigger a shutdown
+# @guest-panic: The guest panicked, and the command line response to
+# a panic is to trigger a shutdown
+#
+# Since: 2.10
+##
+{ 'enum': 'ShutdownCause',
+ 'data': [ 'host-qmp', 'host-signal', 'host-ui', 'host-replay', 'host-error',
+'guest-shutdown', 'guest-reset', 'guest-panic' ] }
+
+##
# @cpu:
#
# This command is a nop that is only provided for the purposes of
compatibility.
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 16175f7..00a907f 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -65,7 +65,7 @@ bool qemu_vmstop_requested(RunState *r);
int qemu_shutdown_requested_get(void);
int qemu_reset_requested_get(void);
void qemu_system_killed(int signal, pid_t pid);
-void qemu_system_reset(bool report);
+void qemu_system_reset(bool report, int reason);
void qemu_system_guest_panicked(GuestPanicInformation *info);
size_t qemu_target_page_size(void);
diff --git a/vl.c b/vl.c
index 879786a..2b95b7f 100644
--- a/vl.c
+++ b/vl.c
@@ -1597,8 +1597,8 @@ void vm_state_notify(int running, RunState state)
}
}
-static int reset_requested;
-static int shutdown_requested, shutdown_signal;
+static int reset_requested = -1;
+static int shutdown_requested = -1, shutdown_signal;
static pid_t shutdown_pid;
static int powerdown_requested;
static int debug_requested;
@@ -1624,7 +1624,7 @@ int qemu_reset_requested_get(void)
static int qemu_shutdown_requested(void)
{
-return atomic_xchg(_requested, 0);
+return atomic_xchg(_requested, -1);
}
static void qemu_kill_report(void)
@@ -1650,11 +1650,11 @@ static void qemu_kill_report(void)
static int qemu_reset_requested(void)
{
int r = reset_requested;
-if (r && replay_checkpoint(CHECKPOINT_RESET_REQUESTED)) {
-reset_requested = 0;
+if (r >= 0 && replay_checkpoint(CHECKPOINT_RESET_REQUESTED)) {
+reset_requested = -1;
return r;
}
-return false;
+return -1;
}
static int qemu_suspend_requested(void)
@@ -1686,7 +1686,12 @@ static int qemu_debug_requested(void)
return r;
}
-void qemu_system_reset(bool report)
+/*
+ * Reset the VM. If @report is VMRESET_REPORT, issue an event, using
+ * the @reason interpreted as ShutdownType for details. Otherwise,
+ * @report is VMRESET_SILENT and @reason is ignored.
+ */
+void qemu_system_reset(bool report, int reason)
{
MachineClass *mc;
@@ -1700,6 +1705,7 @@ void qemu_system_reset(bool report)
qemu_devices_reset();
}
if (report) {
+
[Xen-devel] [PATCH v3 3/4] shutdown: Add source information to SHUTDOWN and RESET
Libvirt would like to be able to distinguish between a SHUTDOWN
event triggered solely by guest request and one triggered by a
SIGTERM or other action on the host. While qemu_kill_report() is
already able to tell whether a shutdown was triggered by a host
signal (but NOT by a host UI event, such as clicking the X on
the window), that information was then lost after being printed
to stderr. The previous patch prepped things to use an enum
internally; now it's time to wire it up through all callers, and
to extend the SHUTDOWN and RESET events to report the details.
Enhance the shutdown request path to take a parameter of which
way it is being triggered, and update ALL callers. It would have
been less churn to keep the common case with no arguments as
meaning guest-triggered, and only modified the host-triggered
code paths, via a wrapper function, but then we'd still have to
audit that I didn't miss any host-triggered spots; changing the
signature forces us to double-check that I correctly categorized
all callers.
Since command line options can change whether a guest reset request
causes an actual reset vs. a shutdown, it's easy to also add the
information to the RESET event, even though libvirt has not yet
expressed a need to know that.
For the moment, we keep the enum ShutdownType for internal use
only, and merely expose a single boolean of 'guest':true|false
to the QMP client; this is because we don't yet have evidence that
the further distinctions will be useful, or whether the addition
of new enum members would cause problems to clients coded to an
older version of the enum.
Update expected iotest outputs to match the new data.
Here is output from 'virsh qemu-monitor-event --loop' with the
patch installed:
event SHUTDOWN at 1492639680.731251 for domain fedora_13: {"guest":true}
event STOP at 1492639680.732116 for domain fedora_13:
event SHUTDOWN at 1492639680.732830 for domain fedora_13: {"guest":false}
Note that libvirt runs qemu with -no-quit: the first SHUTDOWN event
was triggered by an action I took directly in the guest (shutdown -h),
at which point qemu stops the vcpus and waits for libvirt to do any
final cleanups; the second SHUTDOWN event is the result of libvirt
sending SIGTERM now that it has completed cleanup.
The replay driver needs a followup patch if we want to be able to
faithfully replay the difference between a host- and guest-initiated
shutdown (for now, the replayed event is always attributed to host).
See also https://bugzilla.redhat.com/1384007
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
v3: retitle again, fix qemu-iotests, use enum rather than raw bool
in all callers
v2: retitle (was "event: Add signal information to SHUTDOWN"),
completely rework to post bool based on whether it is guest-initiated
v1: initial submission, exposing just Unix signals from host
---
qapi/event.json | 17 +
include/sysemu/sysemu.h | 4 ++--
vl.c| 20 ++--
hw/acpi/core.c | 4 ++--
hw/arm/highbank.c | 4 ++--
hw/arm/integratorcp.c | 2 +-
hw/arm/musicpal.c | 2 +-
hw/arm/omap1.c | 6 +++---
hw/arm/omap2.c | 2 +-
hw/arm/spitz.c | 2 +-
hw/arm/stellaris.c | 2 +-
hw/arm/tosa.c | 2 +-
hw/i386/pc.c| 2 +-
hw/i386/xen/xen-hvm.c | 2 +-
hw/input/pckbd.c| 4 ++--
hw/ipmi/ipmi.c | 4 ++--
hw/isa/lpc_ich9.c | 2 +-
hw/mips/boston.c| 2 +-
hw/mips/mips_malta.c| 2 +-
hw/mips/mips_r4k.c | 4 ++--
hw/misc/arm_sysctl.c| 8
hw/misc/cbus.c | 2 +-
hw/misc/macio/cuda.c| 4 ++--
hw/misc/slavio_misc.c | 4 ++--
hw/misc/zynq_slcr.c | 2 +-
hw/pci-host/apb.c | 4 ++--
hw/pci-host/bonito.c| 2 +-
hw/pci-host/piix.c | 2 +-
hw/ppc/e500.c | 2 +-
hw/ppc/mpc8544_guts.c | 2 +-
hw/ppc/ppc.c| 2 +-
hw/ppc/ppc405_uc.c | 2 +-
hw/ppc/spapr_hcall.c| 2 +-
hw/ppc/spapr_rtas.c | 4 ++--
hw/s390x/ipl.c | 2 +-
hw/sh4/r2d.c| 2 +-
hw/timer/etraxfs_timer.c| 2 +-
hw/timer/m48t59.c | 4 ++--
hw/timer/milkymist-sysctl.c | 4 ++--
hw/timer/pxa2xx_timer.c | 2 +-
hw/watchdog/watchdog.c | 2 +-
hw/xenpv/xen_domainbuild.c | 2 +-
hw/xtensa/xtfpga.c | 2 +-
kvm-all.c | 6 +++---
os-win32.c | 2 +-
qmp.c | 4 ++--
replay/replay.c | 5 -
target/alpha/sys_helper.c | 4 ++--
target/arm/psci.c | 4 ++--
target/i386/excp_helper.c | 2 +-
target/i386/hax-all.c | 6 +++---
target/i386/helper.c| 2 +-
target/i386/kvm.c | 2 +-
target/s390x/helper.c | 2 +-
target/s390x/kv
[Xen-devel] [PATCH v3 2/4] shutdown: Prepare for use of an enum in reset/shutdown_request
We want to track why a guest was shutdown; in particular, being able
to tell the difference between a guest request (such as ACPI request)
and host request (such as SIGINT) will prove useful to libvirt.
Since all requests eventually end up changing shutdown_requested in
vl.c, the logical change is to make that value track the reason,
rather than its current 0/1 contents.
Since command-line options control whether a reset request is turned
into a shutdown request instead, the same treatment is given to
reset_requested.
This patch adds a QAPI enum ShutdownType that describes reasons that
a shutdown can be requested, and changes qemu_system_reset() to pass
the reason through, although for now it is not reported. The next
patch will actually wire things up to modify events to report data
based on the reason, and to pass the correct enum value in from
various call-sites that can trigger a reset/shutdown. Since QAPI
generates enums starting at 0, it's easier if we use a different
number as our sentinel that no request has happened yet. Most of
the changes are in vl.c, but xen was using things externally.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
v3: new patch
---
qapi-schema.json| 23 +++
include/sysemu/sysemu.h | 2 +-
vl.c| 44
hw/i386/xen/xen-hvm.c | 9 ++---
migration/colo.c| 2 +-
migration/savevm.c | 2 +-
6 files changed, 60 insertions(+), 22 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 01b087f..8b9819f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2304,6 +2304,29 @@
{ 'command': 'system_powerdown' }
##
+# @ShutdownType:
+#
+# Enumeration of various causes for shutdown.
+#
+# @host-qmp: Reaction to a QMP command, such as 'quit'
+# @host-signal: Reaction to a signal, such as SIGINT
+# @host-ui: Reaction to a UI event, such as closing the window
+# @host-replay: The host is replaying an earlier shutdown event
+# @host-error: Qemu encountered an error that prevents further use of the guest
+# @guest-shutdown: The guest requested a shutdown, such as via ACPI or
+# other hardware-specific action
+# @guest-reset: The guest requested a reset, and the command line
+# response to a reset is to instead trigger a shutdown
+# @guest-panic: The guest panicked, and the command line response to
+# a panic is to trigger a shutdown
+#
+# Since: 2.10
+##
+{ 'enum': 'ShutdownType',
+ 'data': [ 'host-qmp', 'host-signal', 'host-ui', 'host-replay', 'host-error',
+'guest-shutdown', 'guest-reset', 'guest-panic' ] }
+
+##
# @cpu:
#
# This command is a nop that is only provided for the purposes of
compatibility.
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 16175f7..00a907f 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -65,7 +65,7 @@ bool qemu_vmstop_requested(RunState *r);
int qemu_shutdown_requested_get(void);
int qemu_reset_requested_get(void);
void qemu_system_killed(int signal, pid_t pid);
-void qemu_system_reset(bool report);
+void qemu_system_reset(bool report, int reason);
void qemu_system_guest_panicked(GuestPanicInformation *info);
size_t qemu_target_page_size(void);
diff --git a/vl.c b/vl.c
index 879786a..608cc82 100644
--- a/vl.c
+++ b/vl.c
@@ -1597,8 +1597,8 @@ void vm_state_notify(int running, RunState state)
}
}
-static int reset_requested;
-static int shutdown_requested, shutdown_signal;
+static int reset_requested = -1;
+static int shutdown_requested = -1, shutdown_signal;
static pid_t shutdown_pid;
static int powerdown_requested;
static int debug_requested;
@@ -1624,7 +1624,7 @@ int qemu_reset_requested_get(void)
static int qemu_shutdown_requested(void)
{
-return atomic_xchg(_requested, 0);
+return atomic_xchg(_requested, -1);
}
static void qemu_kill_report(void)
@@ -1650,11 +1650,11 @@ static void qemu_kill_report(void)
static int qemu_reset_requested(void)
{
int r = reset_requested;
-if (r && replay_checkpoint(CHECKPOINT_RESET_REQUESTED)) {
-reset_requested = 0;
+if (r >= 0 && replay_checkpoint(CHECKPOINT_RESET_REQUESTED)) {
+reset_requested = -1;
return r;
}
-return false;
+return -1;
}
static int qemu_suspend_requested(void)
@@ -1686,7 +1686,12 @@ static int qemu_debug_requested(void)
return r;
}
-void qemu_system_reset(bool report)
+/*
+ * Reset the VM. If @report is VMRESET_REPORT, issue an event, using
+ * the @reason interpreted as ShutdownType for details. Otherwise,
+ * @report is VMRESET_SILENT and @reason is ignored.
+ */
+void qemu_system_reset(bool report, int reason)
{
MachineClass *mc;
@@ -1700,6 +1705,7 @@ void qemu_system_reset(bool report)
qemu_devices_reset();
}
if (report) {
+assert(reason >= 0);
qapi_event_send_reset(_abort);
}
cpu_synchronize_
Re: [Xen-devel] [Qemu-devel] [PATCH v2] event: Add source information to SHUTDOWN
On 04/19/2017 05:22 PM, Eric Blake wrote:
> Libvirt would like to be able to distinguish between a SHUTDOWN
> event triggered solely by guest request and one triggered by a
> SIGTERM or other action on the host. qemu_kill_report() is
> already able to tell whether a shutdown was triggered by a host
> signal (but NOT by a host UI event, such as clicking the X on
> the window), but that information was then lost after being
> printed to stderr.
>
> Enhance the shutdown request path to take a parameter of which
> way it is being triggered, and update ALL callers. It would have
> been less churn to keep the common case with no arguments as
> meaning guest-triggered, and only modified the host-triggered
> code paths, via a wrapper function, but then we'd still have to
> audit that I didn't miss any host-triggered spots; changing the
> signature forces us to double-check that I correctly categorized
> all callers.
>
> Here is output from 'virsh qemu-monitor-event --loop' with the
> patch installed:
>
> event SHUTDOWN at 1492639680.731251 for domain fedora_13: {"guest":true}
> event STOP at 1492639680.732116 for domain fedora_13:
> event SHUTDOWN at 1492639680.732830 for domain fedora_13: {"guest":false}
Another reason I'll need a v3: at least qemu-iotests 87 has to be
updated for new expected output.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH v2] event: Add source information to SHUTDOWN
On 04/20/2017 11:18 AM, Markus Armbruster wrote:
> Eric Blake <ebl...@redhat.com> writes:
>
>> On 04/20/2017 06:59 AM, Markus Armbruster wrote:
>>
>>>
>>> No objection to Alistair's idea to turn this into an enumeration.
>>
>> Question - should the enum be more than just 'guest' and 'host'? For
>> example, my patch proves that we have a lot of places that handle
>> complimentary machine commands to reset and shutdown, and that whether
>> 'reset' triggers a reset (and the guest keeps running as if rebooted) or
>> a shutdown is then based on the command-line arguments given to qemu.
>> So having the enum differentiate between 'guest-reset' and
>> 'guest-shutdown' would be a possibility, if we want the enum to have
>> additional states.
>
> I don't know. What I do know is that we better get the enum right:
> while adding members is backwards-compatible, changing the member sent
> for a specific trigger is not. If we want to reserve the option to do
> that anyway, we need suitable documentation.
Or even this idea:
{ 'enum': 'ShutdownCause', 'data': [ 'shutdown', 'reset', 'panic' ] }
{ 'event': 'SHUTDOWN',
'data': { 'guest': 'bool', '*cause': 'ShutdownCause' } }
where the enum can grow as we come up with ever more reasons worth
exposing (maybe even 'qmp', 'gui' and 'interrupt' are reasonable causes
for a host shutdown). Our promise would be that 'guest' never changes
for an existing shutdown reason, but that 'cause' may become more
refined over time if someone expresses a need for having the distinction.
Thoughts?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH v2] event: Add source information to SHUTDOWN
On 04/20/2017 11:12 AM, Markus Armbruster wrote: >>> Well technically /usr/sbin/halt just terminates all processes / kernel and >>> halts CPUs, but the virtual machine is still active (and a 'reset' in the >>> monitor can start it again. /usr/sbin/poweroff is what actually does the >>> ACPI poweroff to trigger QEMU to exit[1] >> >> I'm thinking of this wording: >> >> triggered by a guest request (such as the guest running >> /usr/sbin/poweroff to trigger an ACPI shutdown or machine halt instruction) > > A quick glance at the patch suggests the instructions in question are > typically writes to some device register. I wouldn't call them "halt > instructions", in particular since there's the x86 "hlt" instruction > that does something else. Then maybe: a guest request (such as the guest running /usr/sbin/poweroff to trigger an ACPI or other hardware-specific shutdown sequence) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH v2] event: Add source information to SHUTDOWN
On 04/20/2017 06:59 AM, Markus Armbruster wrote:
>
> No objection to Alistair's idea to turn this into an enumeration.
Question - should the enum be more than just 'guest' and 'host'? For
example, my patch proves that we have a lot of places that handle
complimentary machine commands to reset and shutdown, and that whether
'reset' triggers a reset (and the guest keeps running as if rebooted) or
a shutdown is then based on the command-line arguments given to qemu.
So having the enum differentiate between 'guest-reset' and
'guest-shutdown' would be a possibility, if we want the enum to have
additional states.
>> +++ b/vl.c
>> @@ -1717,7 +1717,7 @@ void qemu_system_guest_panicked(GuestPanicInformation
>> *info)
>> if (!no_shutdown) {
>> qapi_event_send_guest_panicked(GUEST_PANIC_ACTION_POWEROFF,
>> !!info, info, _abort);
>> -qemu_system_shutdown_request();
>> +qemu_system_shutdown_request(false);
>
> Panicking is a guest action. Whether the shutdown on panic should be
> attributed to guest or host is perhaps debatable.
And it relates to the idea that a guest request for a 'reset' turns into
a qemu response of 'shutdown'. After all, whether a guest panic results
in a shutdown action is determined by command-line arguments to qemu.
So if we DO want to differentiate between a guest panic and a normal
guest shutdown, when both events are wired at the command line to cause
the SHUTDOWN action, then that's another enum to add to my list.
>> +++ b/replay/replay.c
>> @@ -51,7 +51,10 @@ bool replay_next_event_is(int event)
>> switch (replay_state.data_kind) {
>> case EVENT_SHUTDOWN:
>> replay_finish_event();
>> -qemu_system_shutdown_request();
>> +/* TODO: track source of shutdown request, to replay a
>> + * guest-initiated request rather than always claiming to
>> + * be from the host? */
>> +qemu_system_shutdown_request(false);
>
> This is what your "need a followup patch" refers to. I'd like to have
> an opinion from someone familiar with replay on what exactly we need
> here.
replay-internal.h has an enum ReplayEvents, which is a list of one-byte
codes used in the replay data stream to record which event to replay. I
don't know if it is easier to change the replay stream to add a 2-byte
code (shutdown-with-cause, followed by an encoding of the cause enum),
or a range of one-byte codes (one new code per number of enum members).
I also don't know how easy or hard it is to extend the enum (are we free
to add events in the middle, or are we worried about back-compat to an
older replay stream that must still play correctly with a newer qemu,
such that new events must be higher than all existing events).
So yes, I'm hoping for feedback from someone familiar with replay.
>
> Amazing number of ways to shut down or reset a machine.
And as I said, it would be easier to submit a patch with less churn by
having the uncommon case (host-triggered) call a new
qemu_system_shutdown_request_reason(enum), while the common case
(guest-triggered) be handled by having qemu_system_shutdown_request()
with no arguments call
qemu_system_shutdown_request_reason(SHUTDOWN_GUEST). I'm just worried
that doing it that way makes it easy for yet another new host shutdown
method to use the wrong wrapper.
>
> Looks sane on first glance.
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH v2] event: Add source information to SHUTDOWN
On 04/20/2017 07:09 AM, Daniel P. Berrange wrote: >>> +++ b/qapi/event.json >>> @@ -10,6 +10,10 @@ >>> # Emitted when the virtual machine has shut down, indicating that qemu is >>> # about to exit. >>> # >>> +# @guest: If true, the shutdown was triggered by a guest request (such as >>> +# executing a halt instruction) rather than a host request (such as sending >>> +# qemu a SIGINT). (since 2.10) >>> +# >> >> "executing a halt instruction" suggests "halt" is a machine instruction. Which is indeed what most of the places I patched to pass 'true' are emulating - the machine halt instruction. >> I think you mean /usr/sbin/halt. Suggest something like "executing a >> halt command". > > Well technically /usr/sbin/halt just terminates all processes / kernel and > halts CPUs, but the virtual machine is still active (and a 'reset' in the > monitor can start it again. /usr/sbin/poweroff is what actually does the > ACPI poweroff to trigger QEMU to exit[1] I'm thinking of this wording: triggered by a guest request (such as the guest running /usr/sbin/poweroff to trigger an ACPI shutdown or machine halt instruction) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH v2] event: Add source information to SHUTDOWN
On 04/19/2017 05:36 PM, Alistair Francis wrote:
> On Wed, Apr 19, 2017 at 3:22 PM, Eric Blake <ebl...@redhat.com> wrote:
>> Libvirt would like to be able to distinguish between a SHUTDOWN
>> event triggered solely by guest request and one triggered by a
>> SIGTERM or other action on the host. qemu_kill_report() is
>> already able to tell whether a shutdown was triggered by a host
>> signal (but NOT by a host UI event, such as clicking the X on
>> the window), but that information was then lost after being
>> printed to stderr.
>>
>> Enhance the shutdown request path to take a parameter of which
>> way it is being triggered, and update ALL callers. It would have
>> been less churn to keep the common case with no arguments as
>> meaning guest-triggered, and only modified the host-triggered
>> code paths, via a wrapper function, but then we'd still have to
>> audit that I didn't miss any host-triggered spots; changing the
>> signature forces us to double-check that I correctly categorized
>> all callers.
> With all this churn is it not worth chaning the bool from_guest to an
> int that we can expand in future?
>
> I'm imagining an enum with multiple values for different shutdown
> events. At the moment it will just be one for guest and one for host,
> but at some point in the future we might want more.
Yes, that makes sense. Probably easiest is creating a QMP enum now ( as
in { 'enum': 'ShutdownCause', 'data': ['guest', 'host'] } - although
such an enum defaults to starting at 0, and the shutdown_requested
variable in vl.c would need a tweak to pick a different value when no
shutdown is requested.
> Not only does this future proof us, but I think it makes it more clear
> what you are passing the function instead of just true/false.
Under your proposal, it would be
qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST), which does look a
bit nicer. I'll wait for other review comments, but you have given me a
good reason to do a v3.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] event: Add source information to SHUTDOWN
Libvirt would like to be able to distinguish between a SHUTDOWN
event triggered solely by guest request and one triggered by a
SIGTERM or other action on the host. qemu_kill_report() is
already able to tell whether a shutdown was triggered by a host
signal (but NOT by a host UI event, such as clicking the X on
the window), but that information was then lost after being
printed to stderr.
Enhance the shutdown request path to take a parameter of which
way it is being triggered, and update ALL callers. It would have
been less churn to keep the common case with no arguments as
meaning guest-triggered, and only modified the host-triggered
code paths, via a wrapper function, but then we'd still have to
audit that I didn't miss any host-triggered spots; changing the
signature forces us to double-check that I correctly categorized
all callers.
Here is output from 'virsh qemu-monitor-event --loop' with the
patch installed:
event SHUTDOWN at 1492639680.731251 for domain fedora_13: {"guest":true}
event STOP at 1492639680.732116 for domain fedora_13:
event SHUTDOWN at 1492639680.732830 for domain fedora_13: {"guest":false}
Note that libvirt runs qemu with -no-quit: the first SHUTDOWN event
was triggered by an action I took directly in the guest (shutdown -h),
at which point qemu stops the vcpus and waits for libvirt to do any
final cleanups; the second SHUTDOWN event is the result of libvirt
sending SIGTERM now that it has completed cleanup.
See also https://bugzilla.redhat.com/1384007
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
I did not wire up the RESET event to report guest-triggered, although
I had to plumb that through all the guests since qemu has options that
allow remapping reset to shutdown. It's easy to add that if we want
it in v3.
The replay driver needs a followup patch if we want to be able to
faithfully replay the difference between a host- and guest-initiated
shutdown (for now, the replayed event is always attributed to host).
v2: retitle (was "event: Add signal information to SHUTDOWN"),
completely rework to post bool based on whether it is guest-initiated
v1: initial submission, exposing just Unix signals from host
---
qapi/event.json | 8 ++--
include/sysemu/sysemu.h | 4 ++--
vl.c| 19 +++
hw/acpi/core.c | 4 ++--
hw/arm/highbank.c | 4 ++--
hw/arm/integratorcp.c | 2 +-
hw/arm/musicpal.c | 2 +-
hw/arm/omap1.c | 6 +++---
hw/arm/omap2.c | 2 +-
hw/arm/spitz.c | 2 +-
hw/arm/stellaris.c | 2 +-
hw/arm/tosa.c | 2 +-
hw/i386/pc.c| 2 +-
hw/input/pckbd.c| 4 ++--
hw/ipmi/ipmi.c | 4 ++--
hw/isa/lpc_ich9.c | 2 +-
hw/mips/boston.c| 2 +-
hw/mips/mips_malta.c| 2 +-
hw/mips/mips_r4k.c | 4 ++--
hw/misc/arm_sysctl.c| 8
hw/misc/cbus.c | 2 +-
hw/misc/macio/cuda.c| 4 ++--
hw/misc/slavio_misc.c | 4 ++--
hw/misc/zynq_slcr.c | 2 +-
hw/pci-host/apb.c | 4 ++--
hw/pci-host/bonito.c| 2 +-
hw/pci-host/piix.c | 2 +-
hw/ppc/e500.c | 2 +-
hw/ppc/mpc8544_guts.c | 2 +-
hw/ppc/ppc.c| 2 +-
hw/ppc/ppc405_uc.c | 2 +-
hw/ppc/spapr_hcall.c| 2 +-
hw/ppc/spapr_rtas.c | 4 ++--
hw/s390x/ipl.c | 2 +-
hw/sh4/r2d.c| 2 +-
hw/timer/etraxfs_timer.c| 2 +-
hw/timer/m48t59.c | 4 ++--
hw/timer/milkymist-sysctl.c | 4 ++--
hw/timer/pxa2xx_timer.c | 2 +-
hw/watchdog/watchdog.c | 2 +-
hw/xenpv/xen_domainbuild.c | 2 +-
hw/xtensa/xtfpga.c | 2 +-
kvm-all.c | 6 +++---
os-win32.c | 2 +-
qmp.c | 4 ++--
replay/replay.c | 5 -
target/alpha/sys_helper.c | 4 ++--
target/arm/psci.c | 4 ++--
target/i386/excp_helper.c | 2 +-
target/i386/hax-all.c | 6 +++---
target/i386/helper.c| 2 +-
target/i386/kvm.c | 2 +-
target/s390x/helper.c | 2 +-
target/s390x/kvm.c | 4 ++--
target/s390x/misc_helper.c | 4 ++--
target/sparc/int32_helper.c | 2 +-
ui/sdl.c| 2 +-
ui/sdl2.c | 4 ++--
xen-hvm.c | 2 +-
trace-events| 2 +-
ui/cocoa.m | 2 +-
61 files changed, 106 insertions(+), 96 deletions(-)
diff --git a/qapi/event.json b/qapi/event.json
index e80f3f4..c230265 100644
--- a/qapi/event.json
+++ b/qapi/event.json
@@ -10,6 +10,10 @@
# Emitted when the virtual machine has shut down, indicating that qemu is
# about to exit.
#
+# @guest: If true, the shutdown was triggered by a guest request (such as
+# executing a halt instruction) rather than a host request (such as se
[Xen-devel] [PATCH v4 09/13] qobject: Use simpler QDict/QList scalar insertion macros
We now have macros in place to make it less verbose to add a scalar
to QDict and QList, so use them. To make this patch smaller to
review, a couple of subdirectories were done in earlier patches.
Patch created mechanically via:
spatch --sp-file scripts/coccinelle/qobject.cocci \
--macro-file scripts/cocci-macro-file.h --dir . --in-place
and needed only one touch-up in monitor.c to avoid a long line.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
v4: no change
v3: new patch
---
block.c | 45 +++--
blockdev.c| 30 +-
hw/block/xen_disk.c | 2 +-
hw/usb/xen-usb.c | 12 ++--
monitor.c | 23 +++
qapi/qmp-event.c | 2 +-
qemu-img.c| 6 +++---
qemu-io.c | 2 +-
qemu-nbd.c| 2 +-
qobject/qdict.c | 2 +-
target/s390x/cpu_models.c | 4 ++--
util/qemu-option.c| 2 +-
12 files changed, 60 insertions(+), 72 deletions(-)
diff --git a/block.c b/block.c
index 9024518..c8a6bce 100644
--- a/block.c
+++ b/block.c
@@ -937,16 +937,14 @@ static void update_flags_from_options(int *flags,
QemuOpts *opts)
static void update_options_from_flags(QDict *options, int flags)
{
if (!qdict_haskey(options, BDRV_OPT_CACHE_DIRECT)) {
-qdict_put(options, BDRV_OPT_CACHE_DIRECT,
- qbool_from_bool(flags & BDRV_O_NOCACHE));
+qdict_put_bool(options, BDRV_OPT_CACHE_DIRECT, flags & BDRV_O_NOCACHE);
}
if (!qdict_haskey(options, BDRV_OPT_CACHE_NO_FLUSH)) {
-qdict_put(options, BDRV_OPT_CACHE_NO_FLUSH,
- qbool_from_bool(flags & BDRV_O_NO_FLUSH));
+qdict_put_bool(options, BDRV_OPT_CACHE_NO_FLUSH,
+ flags & BDRV_O_NO_FLUSH);
}
if (!qdict_haskey(options, BDRV_OPT_READ_ONLY)) {
-qdict_put(options, BDRV_OPT_READ_ONLY,
- qbool_from_bool(!(flags & BDRV_O_RDWR)));
+qdict_put_bool(options, BDRV_OPT_READ_ONLY, !(flags & BDRV_O_RDWR));
}
}
@@ -1362,7 +1360,7 @@ static int bdrv_fill_options(QDict **options, const char
*filename,
/* Fetch the file name from the options QDict if necessary */
if (protocol && filename) {
if (!qdict_haskey(*options, "filename")) {
-qdict_put(*options, "filename", qstring_from_str(filename));
+qdict_put_str(*options, "filename", filename);
parse_filename = true;
} else {
error_setg(errp, "Can't specify 'file' and 'filename' options at "
@@ -1383,7 +1381,7 @@ static int bdrv_fill_options(QDict **options, const char
*filename,
}
drvname = drv->format_name;
-qdict_put(*options, "driver", qstring_from_str(drvname));
+qdict_put_str(*options, "driver", drvname);
} else {
error_setg(errp, "Must specify either driver or file");
return -EINVAL;
@@ -2038,7 +2036,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict
*parent_options,
}
if (bs->backing_format[0] != '\0' && !qdict_haskey(options, "driver")) {
-qdict_put(options, "driver", qstring_from_str(bs->backing_format));
+qdict_put_str(options, "driver", bs->backing_format);
}
backing_hd = bdrv_open_inherit(*backing_filename ? backing_filename : NULL,
@@ -2193,12 +2191,9 @@ static BlockDriverState
*bdrv_append_temp_snapshot(BlockDriverState *bs,
}
/* Prepare options QDict for the temporary file */
-qdict_put(snapshot_options, "file.driver",
- qstring_from_str("file"));
-qdict_put(snapshot_options, "file.filename",
- qstring_from_str(tmp_filename));
-qdict_put(snapshot_options, "driver",
- qstring_from_str("qcow2"));
+qdict_put_str(snapshot_options, "file.driver", "file");
+qdict_put_str(snapshot_options, "file.filename", tmp_filename);
+qdict_put_str(snapshot_options, "driver", "qcow2");
bs_snapshot = bdrv_open(NULL, NULL, snapshot_options, flags, errp);
snapshot_options = NULL;
@@ -2373,8 +2368,7 @@ static BlockDriverState *bdrv_open_inherit(const char
*filename,
goto fail;
}
-qdict_put(options, "file",
- qstring_from_str(bdrv_get_node_name(file_bs)));
+qdict_put_str(options, "file", bdrv_get_node_name(file_bs));
}
}
@@ -2396,8 +2390,8 @@ static BlockDriverState *bdrv_open_inherit(const char
*filename,
* sure to update both bs->options (which has the full effective
* options for bs) and options (which ha
Re: [Xen-devel] [Qemu-devel] [PATCH v3 09/13] qobject: Use simpler QDict/QList scalar insertion macros
On 04/11/2017 12:12 PM, Markus Armbruster wrote:
> Eric Blake <ebl...@redhat.com> writes:
>
>> We now have macros in place to make it less verbose to add a scalar
>> to QDict and QList, so use them. To make this patch smaller to
>> review, a couple of subdirectories were done in earlier patches.
>>
>> Patch created mechanically via:
>> spatch --sp-file scripts/coccinelle/qobject.cocci \
>> --macro-file scripts/cocci-macro-file.h --dir . --in-place
>> and needed only one touch-up in monitor.c to avoid a long line.
>>
>> Signed-off-by: Eric Blake <ebl...@redhat.com>
>> ---
>> block.c | 45 +++--
>> blockdev.c| 30 +-
>> hw/block/xen_disk.c | 2 +-
>> hw/usb/xen-usb.c | 12 ++--
>> monitor.c | 23 +++
>> qapi/qmp-event.c | 2 +-
>> qemu-img.c| 6 +++---
>> qemu-io.c | 2 +-
>> qemu-nbd.c| 2 +-
>> qobject/qdict.c | 2 +-
>> target/s390x/cpu_models.c | 4 ++--
>> util/qemu-option.c| 2 +-
>> 12 files changed, 60 insertions(+), 72 deletions(-)
>
> Looks like you intent is to split the application of qobject.cocci into
> block [PATCH 07], tests [PATCH 08] and rest [PATCH 09, i.e. this one].
> Okay, although everything on one patch would also be okay for me.
> However, this patch also contains block stuff: block.c blockdev.c
> qemu-{img,io,nbd}.c. Move to PATCH 07?
Can do either way, although I'd have to tweak the commit message's
sample command lines I used for the split if you like it kept separate.
Maybe even just a comment in patch 07 that 'only files under block/ are
touched here; other block-related patches will be fixed in the remaining
global cleanup'.
Do you have a preference?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 09/13] qobject: Use simpler QDict/QList scalar insertion macros
We now have macros in place to make it less verbose to add a scalar
to QDict and QList, so use them. To make this patch smaller to
review, a couple of subdirectories were done in earlier patches.
Patch created mechanically via:
spatch --sp-file scripts/coccinelle/qobject.cocci \
--macro-file scripts/cocci-macro-file.h --dir . --in-place
and needed only one touch-up in monitor.c to avoid a long line.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
block.c | 45 +++--
blockdev.c| 30 +-
hw/block/xen_disk.c | 2 +-
hw/usb/xen-usb.c | 12 ++--
monitor.c | 23 +++
qapi/qmp-event.c | 2 +-
qemu-img.c| 6 +++---
qemu-io.c | 2 +-
qemu-nbd.c| 2 +-
qobject/qdict.c | 2 +-
target/s390x/cpu_models.c | 4 ++--
util/qemu-option.c| 2 +-
12 files changed, 60 insertions(+), 72 deletions(-)
diff --git a/block.c b/block.c
index 1803334..9b87bf6 100644
--- a/block.c
+++ b/block.c
@@ -937,16 +937,14 @@ static void update_flags_from_options(int *flags,
QemuOpts *opts)
static void update_options_from_flags(QDict *options, int flags)
{
if (!qdict_haskey(options, BDRV_OPT_CACHE_DIRECT)) {
-qdict_put(options, BDRV_OPT_CACHE_DIRECT,
- qbool_from_bool(flags & BDRV_O_NOCACHE));
+qdict_put_bool(options, BDRV_OPT_CACHE_DIRECT, flags & BDRV_O_NOCACHE);
}
if (!qdict_haskey(options, BDRV_OPT_CACHE_NO_FLUSH)) {
-qdict_put(options, BDRV_OPT_CACHE_NO_FLUSH,
- qbool_from_bool(flags & BDRV_O_NO_FLUSH));
+qdict_put_bool(options, BDRV_OPT_CACHE_NO_FLUSH,
+ flags & BDRV_O_NO_FLUSH);
}
if (!qdict_haskey(options, BDRV_OPT_READ_ONLY)) {
-qdict_put(options, BDRV_OPT_READ_ONLY,
- qbool_from_bool(!(flags & BDRV_O_RDWR)));
+qdict_put_bool(options, BDRV_OPT_READ_ONLY, !(flags & BDRV_O_RDWR));
}
}
@@ -1362,7 +1360,7 @@ static int bdrv_fill_options(QDict **options, const char
*filename,
/* Fetch the file name from the options QDict if necessary */
if (protocol && filename) {
if (!qdict_haskey(*options, "filename")) {
-qdict_put(*options, "filename", qstring_from_str(filename));
+qdict_put_str(*options, "filename", filename);
parse_filename = true;
} else {
error_setg(errp, "Can't specify 'file' and 'filename' options at "
@@ -1383,7 +1381,7 @@ static int bdrv_fill_options(QDict **options, const char
*filename,
}
drvname = drv->format_name;
-qdict_put(*options, "driver", qstring_from_str(drvname));
+qdict_put_str(*options, "driver", drvname);
} else {
error_setg(errp, "Must specify either driver or file");
return -EINVAL;
@@ -2034,7 +2032,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict
*parent_options,
}
if (bs->backing_format[0] != '\0' && !qdict_haskey(options, "driver")) {
-qdict_put(options, "driver", qstring_from_str(bs->backing_format));
+qdict_put_str(options, "driver", bs->backing_format);
}
backing_hd = bdrv_open_inherit(*backing_filename ? backing_filename : NULL,
@@ -2189,12 +2187,9 @@ static BlockDriverState
*bdrv_append_temp_snapshot(BlockDriverState *bs,
}
/* Prepare options QDict for the temporary file */
-qdict_put(snapshot_options, "file.driver",
- qstring_from_str("file"));
-qdict_put(snapshot_options, "file.filename",
- qstring_from_str(tmp_filename));
-qdict_put(snapshot_options, "driver",
- qstring_from_str("qcow2"));
+qdict_put_str(snapshot_options, "file.driver", "file");
+qdict_put_str(snapshot_options, "file.filename", tmp_filename);
+qdict_put_str(snapshot_options, "driver", "qcow2");
bs_snapshot = bdrv_open(NULL, NULL, snapshot_options, flags, errp);
snapshot_options = NULL;
@@ -2369,8 +2364,7 @@ static BlockDriverState *bdrv_open_inherit(const char
*filename,
goto fail;
}
-qdict_put(options, "file",
- qstring_from_str(bdrv_get_node_name(file_bs)));
+qdict_put_str(options, "file", bdrv_get_node_name(file_bs));
}
}
@@ -2392,8 +2386,8 @@ static BlockDriverState *bdrv_open_inherit(const char
*filename,
* sure to update both bs->options (which has the full effective
* options for bs) and options (which has file.* already removed).
Re: [Xen-devel] [Qemu-devel] [PATCH v2 2/6] qdict: Add convenience helpers for wrapped puts
On 04/05/2017 06:54 AM, Paolo Bonzini wrote: > > > On 05/04/2017 11:01, Richard W.M. Jones wrote: >> On Wed, Apr 05, 2017 at 10:38:37AM +0200, Julia Lawall wrote: >>> OK, there is nothing special about g_assert_cmpint, but Coccinelle expects >>> expressions or types in function argument lists, so it gives a parse error >>> on finding an ==. >> >> I should have checked the coccinelle mailing list before asking you, >> because I see that Eric already asked this question and you answered >> it. For reference, that is here: >> >> https://systeme.lip6.fr/pipermail/cocci/2017-April/004107.html >> >> Thanks again, > > Eric, are you using the scripts/cocci-macro-file.h? > > commit 6ad978e9f40d3edfd9f4a86b4a60e3523eff08fe > Author: Paolo Bonzini <pbonz...@redhat.com> > Date: Wed May 18 11:11:55 2016 +0200 > > coccinelle: add g_assert_cmp* to macro file > > This helps applying semantic patches to unit tests. > > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> Didn't even realize it existed. I've updated my spatch command line accordingly. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH v2 2/6] qdict: Add convenience helpers for wrapped puts
On 01/19/2017 08:38 AM, Eric Blake wrote: > On 01/19/2017 03:25 AM, Markus Armbruster wrote: >> Eric Blake <ebl...@redhat.com> writes: >> >>> Quite a few users of qdict_put() were manually wrapping a >>> non-QObject. We can make such call-sites shorter, by providing >>> common macros to do the tedious work. Also shorten nearby >>> qdict_put_obj(,,QOBJECT()) sequences. >>> >>> Signed-off-by: Eric Blake <ebl...@redhat.com> >>> Reviewed-by: Alberto Garcia <be...@igalia.com> >>> >>> --- >>> >>> v2: rebase to current master >>> >>> I'm okay if you want me to break this patch into smaller pieces. >> >> I guess I'm okay with a single piece, but I'd like to know how you did >> the conversion. Coccinelle? Manually? > > Manual, via grepping for put_obj.*QOBJECT. I'll see if I can do the same > under Coccinelle (at which point, committing the script will make it > easier to rerun cleanups if later code reintroduces poor usage > patterns), so maybe I have a v3 coming up. I've got a Coccinelle patch (mostly) working now - but it has one shortfall - I found places in tests/check-qdict.c that coccinelle didn't, and traced it to the fact that our use of g_assert_cmpint(expr, ==, expr) throws off the coccinelle parser so badly that it silently ignores the entire function body containing the use of that macro. v3 will be posted soon, with the best of both worlds (coccinelle caught spots that I missed, not to mention recent code base changes; and my manual search found the spots in tests/ that coccinelle missed). -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH v2 2/6] qdict: Add convenience helpers for wrapped puts
On 01/19/2017 03:25 AM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> writes: > >> Quite a few users of qdict_put() were manually wrapping a >> non-QObject. We can make such call-sites shorter, by providing >> common macros to do the tedious work. Also shorten nearby >> qdict_put_obj(,,QOBJECT()) sequences. >> >> Signed-off-by: Eric Blake <ebl...@redhat.com> >> Reviewed-by: Alberto Garcia <be...@igalia.com> >> >> --- >> >> v2: rebase to current master >> >> I'm okay if you want me to break this patch into smaller pieces. > > I guess I'm okay with a single piece, but I'd like to know how you did > the conversion. Coccinelle? Manually? Manual, via grepping for put_obj.*QOBJECT. I'll see if I can do the same under Coccinelle (at which point, committing the script will make it easier to rerun cleanups if later code reintroduces poor usage patterns), so maybe I have a v3 coming up. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 2/6] qdict: Add convenience helpers for wrapped puts
Quite a few users of qdict_put() were manually wrapping a
non-QObject. We can make such call-sites shorter, by providing
common macros to do the tedious work. Also shorten nearby
qdict_put_obj(,,QOBJECT()) sequences.
Signed-off-by: Eric Blake <ebl...@redhat.com>
Reviewed-by: Alberto Garcia <be...@igalia.com>
---
v2: rebase to current master
I'm okay if you want me to break this patch into smaller pieces.
---
include/qapi/qmp/qdict.h| 8 +++
block.c | 59 +++-
block/archipelago.c | 4 +-
block/blkdebug.c| 6 +-
block/blkverify.c | 11 ++-
block/curl.c| 2 +-
block/file-posix.c | 8 +--
block/file-win32.c | 4 +-
block/iscsi.c | 2 +-
block/nbd.c | 41 ++-
block/nfs.c | 43 +---
block/null.c| 2 +-
block/qcow2.c | 4 +-
block/quorum.c | 13 ++--
block/ssh.c | 16 ++---
block/vvfat.c | 10 +--
blockdev.c | 28
hw/block/xen_disk.c | 2 +-
hw/usb/xen-usb.c| 12 ++--
monitor.c | 18 ++---
qapi/qmp-event.c| 2 +-
qemu-img.c | 6 +-
qemu-io.c | 2 +-
qemu-nbd.c | 2 +-
qobject/qdict.c | 2 +-
target/s390x/cpu_models.c | 4 +-
tests/check-qdict.c | 132 ++--
tests/test-qmp-commands.c | 30
tests/test-qmp-event.c | 30
tests/test-qobject-output-visitor.c | 6 +-
util/qemu-option.c | 6 +-
31 files changed, 245 insertions(+), 270 deletions(-)
diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index fe9a4c5..9d9f9a3 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -52,6 +52,14 @@ void qdict_destroy_obj(QObject *obj);
#define qdict_put(qdict, key, obj) \
qdict_put_obj(qdict, key, QOBJECT(obj))
+/* Helpers for int, bool, and const char*. */
+#define qdict_put_int(qdict, key, value) \
+qdict_put(qdict, key, qint_from_int(value))
+#define qdict_put_bool(qdict, key, value) \
+qdict_put(qdict, key, qbool_from_bool(value))
+#define qdict_put_str(qdict, key, value) \
+qdict_put(qdict, key, qstring_from_str(value))
+
/* High level helpers */
double qdict_get_double(const QDict *qdict, const char *key);
int64_t qdict_get_int(const QDict *qdict, const char *key);
diff --git a/block.c b/block.c
index 39ddea3..e816657 100644
--- a/block.c
+++ b/block.c
@@ -876,16 +876,16 @@ static void update_flags_from_options(int *flags,
QemuOpts *opts)
static void update_options_from_flags(QDict *options, int flags)
{
if (!qdict_haskey(options, BDRV_OPT_CACHE_DIRECT)) {
-qdict_put(options, BDRV_OPT_CACHE_DIRECT,
- qbool_from_bool(flags & BDRV_O_NOCACHE));
+qdict_put_bool(options, BDRV_OPT_CACHE_DIRECT,
+ flags & BDRV_O_NOCACHE);
}
if (!qdict_haskey(options, BDRV_OPT_CACHE_NO_FLUSH)) {
-qdict_put(options, BDRV_OPT_CACHE_NO_FLUSH,
- qbool_from_bool(flags & BDRV_O_NO_FLUSH));
+qdict_put_bool(options, BDRV_OPT_CACHE_NO_FLUSH,
+ flags & BDRV_O_NO_FLUSH);
}
if (!qdict_haskey(options, BDRV_OPT_READ_ONLY)) {
-qdict_put(options, BDRV_OPT_READ_ONLY,
- qbool_from_bool(!(flags & BDRV_O_RDWR)));
+qdict_put_bool(options, BDRV_OPT_READ_ONLY,
+ !(flags & BDRV_O_RDWR));
}
}
@@ -1244,7 +1244,7 @@ static int bdrv_fill_options(QDict **options, const char
*filename,
/* Fetch the file name from the options QDict if necessary */
if (protocol && filename) {
if (!qdict_haskey(*options, "filename")) {
-qdict_put(*options, "filename", qstring_from_str(filename));
+qdict_put_str(*options, "filename", filename);
parse_filename = true;
} else {
error_setg(errp, "Can't specify 'file' and 'filename' options at "
@@ -1264,7 +1264,7 @@ static int bdrv_fill_options(QDict **options, const char
*filename,
}
drvname = drv->format_name;
-qdict_put(*options, "driver", qstring_from_str(drvname));
+qdict_put_str(*options, "driver", drvname);
} else {
error_setg(errp, "Must specify either driver or file");
return -EINVAL;
@@ -1517,7 +1517,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict
*par
[Xen-devel] [PATCH 02/36] qdict: Add convenience helpers for wrapped puts
Quite a few users of qdict_put() were manually wrapping a
non-QObject. We can make such call-sites shorter, by providing
common macros to do the tedious work. Also shorten nearby
qdict_put_obj(,,QOBJECT()) sequences.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
I'm okay if you want me to break this patch into smaller pieces.
---
include/qapi/qmp/qdict.h| 8 +++
block.c | 59 +++-
block/archipelago.c | 4 +-
block/blkdebug.c| 6 +-
block/blkverify.c | 11 ++-
block/curl.c| 2 +-
block/iscsi.c | 2 +-
block/nbd.c | 41 ++-
block/nfs.c | 43 +---
block/null.c| 2 +-
block/qcow2.c | 4 +-
block/quorum.c | 13 ++--
block/raw-posix.c | 8 +--
block/raw-win32.c | 4 +-
block/ssh.c | 16 ++---
block/vvfat.c | 10 +--
blockdev.c | 28
hw/block/xen_disk.c | 2 +-
hw/usb/xen-usb.c| 12 ++--
monitor.c | 18 ++---
qapi/qmp-event.c| 2 +-
qemu-img.c | 6 +-
qemu-io.c | 2 +-
qemu-nbd.c | 2 +-
qobject/qdict.c | 2 +-
target-s390x/cpu_models.c | 4 +-
tests/check-qdict.c | 132 ++--
tests/test-qmp-commands.c | 30
tests/test-qmp-event.c | 30
tests/test-qobject-output-visitor.c | 6 +-
util/qemu-option.c | 6 +-
31 files changed, 245 insertions(+), 270 deletions(-)
diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index fe9a4c5..9d9f9a3 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -52,6 +52,14 @@ void qdict_destroy_obj(QObject *obj);
#define qdict_put(qdict, key, obj) \
qdict_put_obj(qdict, key, QOBJECT(obj))
+/* Helpers for int, bool, and const char*. */
+#define qdict_put_int(qdict, key, value) \
+qdict_put(qdict, key, qint_from_int(value))
+#define qdict_put_bool(qdict, key, value) \
+qdict_put(qdict, key, qbool_from_bool(value))
+#define qdict_put_str(qdict, key, value) \
+qdict_put(qdict, key, qstring_from_str(value))
+
/* High level helpers */
double qdict_get_double(const QDict *qdict, const char *key);
int64_t qdict_get_int(const QDict *qdict, const char *key);
diff --git a/block.c b/block.c
index 39ddea3..e816657 100644
--- a/block.c
+++ b/block.c
@@ -876,16 +876,16 @@ static void update_flags_from_options(int *flags,
QemuOpts *opts)
static void update_options_from_flags(QDict *options, int flags)
{
if (!qdict_haskey(options, BDRV_OPT_CACHE_DIRECT)) {
-qdict_put(options, BDRV_OPT_CACHE_DIRECT,
- qbool_from_bool(flags & BDRV_O_NOCACHE));
+qdict_put_bool(options, BDRV_OPT_CACHE_DIRECT,
+ flags & BDRV_O_NOCACHE);
}
if (!qdict_haskey(options, BDRV_OPT_CACHE_NO_FLUSH)) {
-qdict_put(options, BDRV_OPT_CACHE_NO_FLUSH,
- qbool_from_bool(flags & BDRV_O_NO_FLUSH));
+qdict_put_bool(options, BDRV_OPT_CACHE_NO_FLUSH,
+ flags & BDRV_O_NO_FLUSH);
}
if (!qdict_haskey(options, BDRV_OPT_READ_ONLY)) {
-qdict_put(options, BDRV_OPT_READ_ONLY,
- qbool_from_bool(!(flags & BDRV_O_RDWR)));
+qdict_put_bool(options, BDRV_OPT_READ_ONLY,
+ !(flags & BDRV_O_RDWR));
}
}
@@ -1244,7 +1244,7 @@ static int bdrv_fill_options(QDict **options, const char
*filename,
/* Fetch the file name from the options QDict if necessary */
if (protocol && filename) {
if (!qdict_haskey(*options, "filename")) {
-qdict_put(*options, "filename", qstring_from_str(filename));
+qdict_put_str(*options, "filename", filename);
parse_filename = true;
} else {
error_setg(errp, "Can't specify 'file' and 'filename' options at "
@@ -1264,7 +1264,7 @@ static int bdrv_fill_options(QDict **options, const char
*filename,
}
drvname = drv->format_name;
-qdict_put(*options, "driver", qstring_from_str(drvname));
+qdict_put_str(*options, "driver", drvname);
} else {
error_setg(errp, "Must specify either driver or file");
return -EINVAL;
@@ -1517,7 +1517,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict
*parent_options,
}
if (bs->backing_format[0] != '\0' && !qdict_haskey
Re: [Xen-devel] [Qemu-devel] [PATCH for-2.8 v3] xen_disk: split discard input to match internal representation
On 11/23/2016 04:39 AM, Olaf Hering wrote:
> The guest sends discard requests as u64 sector/count pairs, but the
> block layer operates internally with s64/s32 pairs. The conversion
> leads to IO errors in the guest, the discard request is not processed.
>
> domU.cfg:
> 'vdev=xvda, format=qcow2, backendtype=qdisk, target=/x.qcow2'
> domU:
> mkfs.ext4 -F /dev/xvda
> Discarding device blocks: failed - Input/output error
>
> Fix this by splitting the request into chunks of BDRV_REQUEST_MAX_SECTORS.
> Add input range checking to avoid overflow.
>
> Fixes f313520 ("xen_disk: add discard support")
>
> Signed-off-by: Olaf Hering <o...@aepfle.de>
> ---
Qualifies as a bug fix, so requesting 2.8 inclusion.
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges
On 11/22/2016 11:00 AM, Olaf Hering wrote:
> On Tue, Nov 22, Eric Blake wrote:
>
>> if (sec_start + sec_count < sec_count ||
>> sec_start > (INT64_MAX >> BDRV_SECTOR_BITS) - sec_count) {
>> return false;
>> }
>
> My point was: how does this handle sec_start=0,sec_count=UINT64_MAX or
> sec_start=INT64_MAX,sec_count=INT64_MAX? For me this gets past the if().
Fair enough. Your test that things didn't overflow means that you can
then safely compare the sum to the limit, so go with:
if (sec_start + sec_count < sec_count ||
sec_start + sec_count > INT64_MAX >> BDRV_SECTOR_BITS) {
return false;
}
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges
On 11/22/2016 10:12 AM, Olaf Hering wrote:
> On Fri, Nov 18, Eric Blake wrote:
>
>> if (sec_start > (INT64_MAX >> BDRV_SECTOR_BITS) - sec_count)
>
> I have looked at this for a while now and cant spot how this would cover
> all cases. Are you saying there should be just a single overflow check,
> yours? My change has two: one to check for wrap around and to check
> against the upper limit. My check happens to work with 0/UINT64_MAX or
> INT64_MAX/INT64_MAX as input, yours appearently not.
> Obviously I'm missing something essential.
I never suggested eliminating the wraparound check, only simplifying the
overflow check. You could combine the wraparound and overflow into one:
if (sec_start + sec_count < sec_count ||
sec_start > (INT64_MAX >> BDRV_SECTOR_BITS) - sec_count) {
return false;
}
Remember, sec_start and sec_count were both typed as unsigned 64-bit
values, so everything in the above computation is well-defined
arithmetic, and you catch all cases of trying to add two numbers into
something that doesn't fit in 64 bits, as well as all cases of the
addition fitting in 64 bits but going beyond the maximum possible sector
number (since it is not possible to have a sector number whose
corresponding offset would exceed 63 bits).
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges
On 11/18/2016 11:41 AM, Olaf Hering wrote:
> On Fri, Nov 18, Eric Blake wrote:
>
>> On 11/18/2016 04:24 AM, Olaf Hering wrote:
>>> +/* Overflowing byte limit? */
>>> +if ((sec_start + sec_count) > ((INT64_MAX + INT_MAX) >>
>>> BDRV_SECTOR_BITS)) {
>> This is undefined. INT64_MAX + anything non-negative overflows int64,
>
> The expanded value used to be stored into a uint64_t before it was used
> here. A "cleanup" introduced this error. Thanks for spotting.
>
>> If you are trying to detect guests that make a request that would cover
>> more than INT64_MAX bytes, you can simplify. Besides, for as much
>> storage as there is out there, I seriously doubt ANYONE will ever have
>> 2^63 bytes addressable through a single device. Why not just write it as:
>>
>> if ((INT64_MAX >> BDRV_SECTOR_BITS) - sec_count < sec_start) {
>
> That would always be false I think. I will resubmit with this:
> if ((sec_start + sec_count) > (INT64_MAX >> BDRV_SECTOR_BITS)) {
You're testing whether something overflows, but you don't want to cause
overflow as part of the test. So use the commutative law to rewrite it
to avoid sec_start+sec_count from overflowing, and you get:
if (sec_start > (INT64_MAX >> BDRV_SECTOR_BITS) - sec_count)
but that's exactly the expression I wrote above.
>
> Regarding the cast for ->req, it has type blkif_request_t, but the
> pointer needs to be assigned to type blkif_request_discard_t.
Then why is the cast to (void*) instead of (blkif_request_discard_t*) ?
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges
On 11/18/2016 04:24 AM, Olaf Hering wrote:
> The guest sends discard requests as u64 sector/count pairs, but the
> block layer operates internally with s64/s32 pairs. The conversion
> leads to IO errors in the guest, the discard request is not processed.
>
> domU.cfg:
> 'vdev=xvda, format=qcow2, backendtype=qdisk, target=/x.qcow2'
> domU:
> mkfs.ext4 -F /dev/xvda
> Discarding device blocks: failed - Input/output error
>
> Fix this by splitting the request into chunks of BDRV_REQUEST_MAX_SECTORS.
> Add input range checking to avoid overflow.
>
> Signed-off-by: Olaf Hering <o...@aepfle.de>
> ---
> hw/block/xen_disk.c | 45 +++--
> 1 file changed, 39 insertions(+), 6 deletions(-)
>
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 3a7dc19..c3f572f 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -660,6 +660,41 @@ static void qemu_aio_complete(void *opaque, int ret)
> qemu_bh_schedule(ioreq->blkdev->bh);
> }
>
> +static bool blk_split_discard(struct ioreq *ioreq, blkif_sector_t
> sector_number,
> + uint64_t nr_sectors)
> +{
> +struct XenBlkDev *blkdev = ioreq->blkdev;
> +int64_t byte_offset;
> +int byte_chunk;
> +uint64_t sec_start = sector_number;
> +uint64_t sec_count = nr_sectors;
> +uint64_t byte_remaining;
> +uint64_t limit = BDRV_REQUEST_MAX_SECTORS << BDRV_SECTOR_BITS;
[For reference, this limit is the same as rounding INT32_MAX down to the
nearest 512-byte limit, or 0x7e00]
> +
> +/* Wrap around? */
> +if ((sec_start + sec_count) < sec_count) {
> +return false;
> +}
> +/* Overflowing byte limit? */
> +if ((sec_start + sec_count) > ((INT64_MAX + INT_MAX) >>
> BDRV_SECTOR_BITS)) {
This is undefined. INT64_MAX + anything non-negative overflows int64,
and even if you treat overflow as defined by twos-complement
representation (which creates a negative number), shifting a negative
number is also undefined.
If you are trying to detect guests that make a request that would cover
more than INT64_MAX bytes, you can simplify. Besides, for as much
storage as there is out there, I seriously doubt ANYONE will ever have
2^63 bytes addressable through a single device. Why not just write it as:
if ((INT64_MAX >> BDRV_SECTOR_BITS) - sec_count < sec_start) {
> +return false;
> +}
> +
> +byte_offset = sec_start << BDRV_SECTOR_BITS;
> +byte_remaining = sec_count << BDRV_SECTOR_BITS;
> +
> +do {
> +byte_chunk = byte_remaining > limit ? limit : byte_remaining;
> +ioreq->aio_inflight++;
> +blk_aio_pdiscard(blkdev->blk, byte_offset, byte_chunk,
> + qemu_aio_complete, ioreq);
> +byte_remaining -= byte_chunk;
> +byte_offset += byte_chunk;
> +} while (byte_remaining > 0);
This part looks reasonable.
> +
> +return true;
> +}
> +
> static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
> {
> struct XenBlkDev *blkdev = ioreq->blkdev;
> @@ -708,12 +743,10 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
> break;
> case BLKIF_OP_DISCARD:
> {
> -struct blkif_request_discard *discard_req = (void *)>req;
The old code had it...
> -ioreq->aio_inflight++;
> -blk_aio_pdiscard(blkdev->blk,
> - discard_req->sector_number << BDRV_SECTOR_BITS,
> - discard_req->nr_sectors << BDRV_SECTOR_BITS,
> - qemu_aio_complete, ioreq);
> +struct blkif_request_discard *req = (void *)>req;
...but C doesn't require a cast to void*. As long as you are touching
this, you could remove the cast (unless I'm missing something, and the
cast is also there to cast away const).
> +if (!blk_split_discard(ioreq, req->sector_number, req->nr_sectors)) {
> +goto err;
> +}
> break;
> }
> default:
>
>
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges
On 11/18/2016 08:19 AM, Olaf Hering wrote: > Am 18. November 2016 14:43:18 MEZ, schrieb Eric Blake <ebl...@redhat.com>: >> On 11/18/2016 04:24 AM, Olaf Hering wrote: >>> The guest sends discard requests as u64 sector/count pairs, but the >>> block layer operates internally with s64/s32 pairs. The conversion >>> leads to IO errors in the guest, the discard request is not >> processed. >> >> Doesn't the block layer already split discard requests into 2^31 byte >> chunks? > > How would it do that without valid input? It was wrong before the sectors to > bytes conversion, and now its even worse given that all the world fits into > an int. Then it sounds like the real bug is that the block layer bdrv_co_pdiscard() is buggy for taking 'int count' instead of 'uint64_t count'. Eventually, I think the entire block layer should be fixed to allow 64-bit count everywhere, and then auto-fragment it back down to 31 bits (or even smaller, like NBD's 32M limit or Linux loopback device 64k limit) as needed, rather than making all the backends reimplement fragmentation. > > Remember that there is no API to let the guest know about the limitations of > the host. Correct. But the goal of the block layer is to hide the quirks, so that the code handling the guest requests can offload all the common work to one place. Kevin, is it too late for 2.8 for patches that change bdrv_co_pdiscard to take a 64-bit count? Or would that still be under bug-fix category because of the xen use case? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges
On 11/18/2016 04:24 AM, Olaf Hering wrote:
> The guest sends discard requests as u64 sector/count pairs, but the
> block layer operates internally with s64/s32 pairs. The conversion
> leads to IO errors in the guest, the discard request is not processed.
Doesn't the block layer already split discard requests into 2^31 byte
chunks?
>
> domU.cfg:
> 'vdev=xvda, format=qcow2, backendtype=qdisk, target=/x.qcow2'
> domU:
> mkfs.ext4 -F /dev/xvda
> Discarding device blocks: failed - Input/output error
>
> Fix this by splitting the request into chunks of BDRV_REQUEST_MAX_SECTORS.
> Add input range checking to avoid overflow.
>
> Signed-off-by: Olaf Hering <o...@aepfle.de>
> ---
> hw/block/xen_disk.c | 45 +++--
> 1 file changed, 39 insertions(+), 6 deletions(-)
>
> @@ -708,12 +743,10 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
> break;
> case BLKIF_OP_DISCARD:
> {
> -struct blkif_request_discard *discard_req = (void *)>req;
> -ioreq->aio_inflight++;
> -blk_aio_pdiscard(blkdev->blk,
That is, blk_aio_pdiscard() calls into bdrv_co_pdiscard() which is
supposed to be fragmenting things as needed. Can you trace what is
going wrong there? You shouldn't have to reimplement fragementation if
the block layer is doing it correctly.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [RFC QEMU PATCH 8/8] qmp: add a qmp command 'query-nvdimms' to get plugged NVDIMM devices
On 10/11/2016 03:22 AM, Markus Armbruster wrote:
> query-memory-devices returns a list of MemoryDeviceInfo:
>
> ##
> # @MemoryDeviceInfo:
> #
> # Union containing information about a memory device
> #
> # Since: 2.1
> ##
> { 'union': 'MemoryDeviceInfo', 'data': {'dimm': 'PCDIMMDeviceInfo'} }
>
> This is a union, designed to be extended for other types of memory
> device frontends.
>
> Sadly, it's a "simple" union (I dislike those).
>
> You could add a new member to be used for the NVDIMM case. It would
> probably duplicate some or all of PCDIMMDeviceInfo, though.
>
> If it needs all of PCDIMMDeviceInfo, you could make the new member's
> type have PCDIMMDeviceInfo as base.
>
> If we can identify information *any* memory device frontend should have,
> the appropriate design would be a flat union with common information
> common, and type-specific information in union branches. Could
> MemoryDeviceInfo be backward-compatibly morphed into such a type?
Yes, conversion to a flat union is possible without breaking existing
QMP usage. It would look something like this (with anonymous base and
branch classes, which is still one of my pending qapi patches to post):
{ 'enum': 'MemoryDeviceType', 'data': [ 'dimm', 'nvdimm' ] }
{ 'union': 'MemoryDeviceInfo', 'base': { 'type': 'MemoryDeviceType' },
'discriminator': 'type',
'data': { 'dimm': { 'data': 'PCIMMDeviceInfo' },
'nvdimm': 'whatever_type_here' } }
We would still have back-compatible:
{ "type": "dimm", "data": { "addr":..., "size":..., ... } }
for dimm, and for nvdimm, we would have
{ "type": "nvdimm", whatever fields we want here }
whether we want all the fields to be flattened in the nvdimm case, vs.
nested (for back-compat) under a 'data' dict in the 'dimm' case, or
whether we want both uses to be nested under a 'data' dict for
consistency, is a matter of taste.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC QEMU PATCH 8/8] qmp: add a qmp command 'query-nvdimms' to get plugged NVDIMM devices
On 10/09/2016 07:34 PM, Haozhong Zhang wrote:
> Xen uses this command to get the backend resource, guest SPA and size of
> NVDIMM devices so as to map them to guest.
>
> Signed-off-by: Haozhong Zhang <haozhong.zh...@intel.com>
> ---
> Cc: Markus Armbruster <arm...@redhat.com>
> +++ b/docs/qmp-commands.txt
> @@ -3800,3 +3800,39 @@ Example for pc machine type started with
> "props": {"core-id": 0, "socket-id": 0, "thread-id": 0}
> }
> ]}
> +
> +EQMP
> +
> +{
> +.name = "query-nvdimms",
> +.args_type = "",
> +.mhandler.cmd_new = qmp_marshal_query_nvdimms,
Needs rebasing - we no longer need SQMP/EQMP sections or callouts to the
initializers, now that commit bd6092e4 has automated the mapping of QAPI
to command registration.
> +},
> +
> +SQMP
> +Show plugged NVDIMM devices
> +---
> +
> +Arguments: None.
> +
> +Example for pc machine type started with
> +-object memory-backend-file,id=mem1,mem-path=/path/to/nvm1,size=4G
> +-device nvdimm,id=nvdimm1,memdev=mem1
> +-object memory-backend-file,id=mem2,mem-path=/path/to/nvm2,size=8G
> +-device nvdimm,id=nvdimm2,memdev=mem2:
> +
> +-> { "execute": "query-nvdimms" }
> +<- { "returns": [
> + {
> + "mem-path": "/path/to/nvm1",
> + "slot": 0,
TAB damage; please fix.
> + "spa": 17179869184,
> + "length": 4294967296
> + },
> + {
> + "mem-path": "/path/to/nvm2",
> + "slot": 1,
> + "spa": 21474836480,
> + "length": 8589934592
> + }
> + ]}
> +++ b/qapi-schema.json
> @@ -4646,3 +4646,32 @@
> # Since: 2.7
> ##
> { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
> +
> +##
> +# @NvdimmInfo
> +#
> +# Information about an NVDIMM device.
> +#
> +# @mem-path: the backend file of the NVDIMM device
> +#
> +# @slot: the slot index of the NVDIMM device
> +#
> +# @spa: the 64-bit SPA base address of the NVDIMM device
> +#
> +# @length: the 64-bit size in bytes of the NVDIMM device
> +#
> +# Since 2.8
> +##
> +{ 'struct': 'NvdimmInfo',
> + 'data': {'mem-path' : 'str', 'slot': 'int', 'spa': 'int', 'length': 'int'}
> }
> +
> +##
> +# @query-nvdimms:
> +#
> +# Returns information about each NVDIMM device
> +#
> +# Returns: a list of @NvdimmInfo for each device
> +#
> +# Since: 2.8
> +##
> +{ 'command': 'query-nvdimms', 'returns': ['NvdimmInfo'] }
>
Is this something that can be added to the existing query-memdev or
query-memory-devices command, instead of needing a new command?
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] xenpv: Fix qemu_uuid compiling error
On 09/27/2016 04:20 AM, Fam Zheng wrote:
> 9c5ce8db2 switched the type of qemu_uuid and this should have followed.
> Fix it.
>
> Signed-off-by: Fam Zheng <f...@redhat.com>
> ---
> hw/xenpv/xen_domainbuild.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Eric Blake <ebl...@redhat.com>
>
> diff --git a/hw/xenpv/xen_domainbuild.c b/hw/xenpv/xen_domainbuild.c
> index b439b0e..457a897 100644
> --- a/hw/xenpv/xen_domainbuild.c
> +++ b/hw/xenpv/xen_domainbuild.c
> @@ -232,7 +232,7 @@ int xen_domain_build_pv(const char *kernel, const char
> *ramdisk,
> unsigned long xenstore_mfn = 0, console_mfn = 0;
> int rc;
>
> -memcpy(uuid, qemu_uuid, sizeof(uuid));
> +memcpy(uuid, _uuid, sizeof(uuid));
> rc = xen_domain_create(xen_xc, ssidref, uuid, flags, _domid);
> if (rc < 0) {
> fprintf(stderr, "xen: xc_domain_create() failed\n");
>
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 04/19] xen: Move evtchn functions to xen_pvdev.c
On 07/25/2016 07:53 AM, Anthony PERARD wrote: > On Sun, Jul 10, 2016 at 02:47:35PM +0300, Emil Condrea wrote: >> The name of the functions moved: >> * xen_be_evtchn_event >> * xen_be_unbind_evtchn >> * xen_be_send_notify >> >> Signed-off-by: Emil Condrea <emilcond...@gmail.com> >> --- >> hw/xen/xen_backend.c | 37 + >> hw/xen/xen_pvdev.c | 35 +++ >> include/hw/xen/xen_backend.h | 2 -- >> include/hw/xen/xen_pvdev.h | 4 >> 4 files changed, 40 insertions(+), 38 deletions(-) >> >> diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c >> index 0a9f9bb..5f2821a 100644 >> --- a/hw/xen/xen_backend.c >> +++ b/hw/xen/xen_backend.c >> @@ -693,4 +658,4 @@ static void xenbe_register_types(void) >> type_register_static(_info); >> } >> >> -type_init(xenbe_register_types); >> +type_init(xenbe_register_types); >> \ No newline at end of file > > Looks like this change does not belong to this patch. For that matter, we prefer that all text files in qemu.git end in a newline (since according to POSIX, a non-empty file that does not end in newline is not a text file). -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 01/19] xen: Create a new file xen_pvdev.c
On 07/17/2016 01:41 AM, Quan Xu wrote:
>
> [Quan:]: comment starts with [Quan:]
>
This line doesn't belong in a commit message; it's fine to put it after
the --- separator though, if it aids mailing list reviewers.
>
> The purpose of the new file is to store generic functions shared by
> frontendand backends such as xenstore operations, xendevs.
>
s/frontendand/front end and/
Please wrap your commit message lines. Since 'git log' displays logs
with indentation, wrapping around 72 characters is ideal.
> Signed-off-by: Quan Xu <quan.xu@x>
> Signed-off-by: Emil Condrea <emilcondrea@x>
These are not valid S-o-b, therefore this patch cannot be applied as-is.
> -int xenstore_read_int(const char *base, const char *node, int *ival)
> -{
> -char *val;
> -int rc = -1;
> -
> -val = xenstore_read_str(base, node);
> [Quan:]: IMO, it is better to initialize val when declares. the same
> comment for the other 'val'
> -if (val && 1 == sscanf(val, "%d", ival)) {
This is not a valid patch. Are you replying to a patch that someone
else posted? If so, your quoting style is VERY difficult to read.
Please consider using a leading > before every line that you are quoting
(rather than pasting it verbatim as if you had written it), and include
a blank line both before and after every line that you insert, to call
visual attention to what is your reply vs. what you are quoting.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 05/19] block: Convert BB interface to byte-based discards
Change sector-based blk_discard(), blk_co_discard(), and
blk_aio_discard() to instead be byte-based blk_pdiscard(),
blk_co_pdiscard(), and blk_aio_pdiscard(). NBD gets a lot
simpler now that ignoring the unaligned portion of a
byte-based discard request is handled under the hood by
the block layer.
Signed-off-by: Eric Blake <ebl...@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>
---
v2: tweak commit message for grep'ability
---
include/sysemu/block-backend.h | 9 -
block/block-backend.c | 25 +++--
block/mirror.c | 5 +++--
hw/block/xen_disk.c| 7 ---
hw/ide/core.c | 6 --
hw/scsi/scsi-disk.c| 8
nbd/server.c | 19 +--
qemu-io-cmds.c | 3 +--
8 files changed, 36 insertions(+), 46 deletions(-)
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 3c3e82f..2da4905 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -139,15 +139,14 @@ BlockAIOCB *blk_aio_pwritev(BlockBackend *blk, int64_t
offset,
BlockCompletionFunc *cb, void *opaque);
BlockAIOCB *blk_aio_flush(BlockBackend *blk,
BlockCompletionFunc *cb, void *opaque);
-BlockAIOCB *blk_aio_discard(BlockBackend *blk,
-int64_t sector_num, int nb_sectors,
-BlockCompletionFunc *cb, void *opaque);
+BlockAIOCB *blk_aio_pdiscard(BlockBackend *blk, int64_t offset, int count,
+ BlockCompletionFunc *cb, void *opaque);
void blk_aio_cancel(BlockAIOCB *acb);
void blk_aio_cancel_async(BlockAIOCB *acb);
int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf);
BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
BlockCompletionFunc *cb, void *opaque);
-int blk_co_discard(BlockBackend *blk, int64_t sector_num, int nb_sectors);
+int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int count);
int blk_co_flush(BlockBackend *blk);
int blk_flush(BlockBackend *blk);
int blk_flush_all(void);
@@ -207,7 +206,7 @@ int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk,
int64_t offset,
int blk_write_compressed(BlockBackend *blk, int64_t sector_num,
const uint8_t *buf, int nb_sectors);
int blk_truncate(BlockBackend *blk, int64_t offset);
-int blk_discard(BlockBackend *blk, int64_t sector_num, int nb_sectors);
+int blk_pdiscard(BlockBackend *blk, int64_t offset, int count);
int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf,
int64_t pos, int size);
int blk_load_vmstate(BlockBackend *blk, uint8_t *buf, int64_t pos, int size);
diff --git a/block/block-backend.c b/block/block-backend.c
index 8b16b95..effa038 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1065,17 +1065,16 @@ BlockAIOCB *blk_aio_flush(BlockBackend *blk,
return bdrv_aio_flush(blk_bs(blk), cb, opaque);
}
-BlockAIOCB *blk_aio_discard(BlockBackend *blk,
-int64_t sector_num, int nb_sectors,
-BlockCompletionFunc *cb, void *opaque)
+BlockAIOCB *blk_aio_pdiscard(BlockBackend *blk,
+ int64_t offset, int count,
+ BlockCompletionFunc *cb, void *opaque)
{
-int ret = blk_check_request(blk, sector_num, nb_sectors);
+int ret = blk_check_byte_request(blk, offset, count);
if (ret < 0) {
return blk_abort_aio_request(blk, cb, opaque, ret);
}
-return bdrv_aio_pdiscard(blk_bs(blk), sector_num << BDRV_SECTOR_BITS,
- nb_sectors << BDRV_SECTOR_BITS, cb, opaque);
+return bdrv_aio_pdiscard(blk_bs(blk), offset, count, cb, opaque);
}
void blk_aio_cancel(BlockAIOCB *acb)
@@ -1107,15 +1106,14 @@ BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned
long int req, void *buf,
return bdrv_aio_ioctl(blk_bs(blk), req, buf, cb, opaque);
}
-int blk_co_discard(BlockBackend *blk, int64_t sector_num, int nb_sectors)
+int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int count)
{
-int ret = blk_check_request(blk, sector_num, nb_sectors);
+int ret = blk_check_byte_request(blk, offset, count);
if (ret < 0) {
return ret;
}
-return bdrv_co_pdiscard(blk_bs(blk), sector_num << BDRV_SECTOR_BITS,
-nb_sectors << BDRV_SECTOR_BITS);
+return bdrv_co_pdiscard(blk_bs(blk), offset, count);
}
int blk_co_flush(BlockBackend *blk)
@@ -1506,15 +1504,14 @@ int blk_truncate(BlockBackend *blk, int64_t offset)
return bdrv_truncate(blk_bs(blk), offset);
}
-int blk_discard(BlockBackend *blk, int64_t sector_num, int nb_sectors)
+int blk_pdiscard(BlockBackend *blk, int64_t offset, int count)
{
-int ret = blk_check_request(
[Xen-devel] [PATCH v9 12/17] qapi: Change Netdev into a flat union
This is a mostly-mechanical conversion that creates a new flat
union 'Netdev' QAPI type that covers all the branches of the
former 'NetClientOptions' simple union, where the branches are
now listed in a new 'NetClientDriver' enum rather than generated
from the simple union. The existence of a flat union has no
change to the command line syntax accepted for new code, and
will make it possible for a future patch to switch the QMP
command to parse a boxed union for no change to valid QMP; but
it does have some ripple effect on the C code when dealing with
the new types.
While making the conversion, note that the 'NetLegacy' type
remains unchanged: it applies only to legacy command line options,
and will not be ported to QMP, so it should remain a wrapper
around a simple union; to avoid confusion, the type named
'NetClientOptions' is now gone, and we introduce 'NetLegacyOptions'
in its place. Then, in the C code, we convert from NetLegacy to
Netdev as soon as possible, so that the bulk of the net stack
only has to deal with one QAPI type, not two. Note that since
the old legacy code always rejected 'hubport', we can just omit
that branch from the new 'NetLegacyOptions' simple union.
Based on an idea originally by Zoltán Kővágó <dirty.ice...@gmail.com>:
Message-Id:
<01a527fbf1a5de880091f98cf011616a78ad.1441627176.git.dirty.ice...@gmail.com>
although the sed script in that patch no longer applies due to
other changes in the tree since then, and I also did some manual
cleanups (such as fixing whitespace to keep checkpatch happy).
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
v8: rewrite commit message, claim authorship, rebase to latest
v7: rebase to latest master
v6: rebase to latest master
---
qapi-schema.json | 49 ++-
include/net/net.h| 4 +-
hw/arm/musicpal.c| 2 +-
hw/core/qdev-properties-system.c | 2 +-
hw/net/allwinner_emac.c | 2 +-
hw/net/cadence_gem.c | 2 +-
hw/net/dp8393x.c | 2 +-
hw/net/e1000.c | 2 +-
hw/net/e1000e.c | 2 +-
hw/net/eepro100.c| 2 +-
hw/net/etraxfs_eth.c | 2 +-
hw/net/fsl_etsec/etsec.c | 2 +-
hw/net/imx_fec.c | 2 +-
hw/net/lan9118.c | 2 +-
hw/net/lance.c | 2 +-
hw/net/mcf_fec.c | 2 +-
hw/net/milkymist-minimac2.c | 2 +-
hw/net/mipsnet.c | 2 +-
hw/net/ne2000-isa.c | 2 +-
hw/net/ne2000.c | 2 +-
hw/net/opencores_eth.c | 2 +-
hw/net/pcnet-pci.c | 2 +-
hw/net/rocker/rocker_fp.c| 2 +-
hw/net/rtl8139.c | 2 +-
hw/net/smc91c111.c | 2 +-
hw/net/spapr_llan.c | 2 +-
hw/net/stellaris_enet.c | 2 +-
hw/net/vhost_net.c | 20 +++---
hw/net/virtio-net.c | 10 +--
hw/net/vmxnet3.c | 2 +-
hw/net/xen_nic.c | 2 +-
hw/net/xgmac.c | 2 +-
hw/net/xilinx_axienet.c | 2 +-
hw/net/xilinx_ethlite.c | 2 +-
hw/usb/dev-network.c | 2 +-
monitor.c| 14 ++---
net/dump.c | 6 +-
net/filter.c | 2 +-
net/hub.c| 22 +++
net/l2tpv3.c | 6 +-
net/net.c| 133 +--
net/netmap.c | 4 +-
net/slirp.c | 6 +-
net/socket.c | 8 +--
net/tap-win32.c | 6 +-
net/tap.c| 24 +++
net/vde.c| 6 +-
net/vhost-user.c | 20 +++---
48 files changed, 230 insertions(+), 172 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index d2d6506..5658723 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2809,16 +2809,32 @@
'*queues':'int' } }
##
-# @NetClientOptions
+# @NetClientDriver
#
-# A discriminated record of network device traits.
+# Available netdev drivers.
+#
+# Since 2.7
+##
+{ 'enum': 'NetClientDriver',
+ 'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde', 'dump',
+'bridge', 'hubport', 'netmap', 'vhost-user' ] }
+
+##
+# @Netdev
+#
+# Captures the configuration of a network device.
+#
+# @id: identifier for monitor commands.
+#
+# @type: Specify the driver used for interpreting remaining arguments.
#
# Since 1.2
#
# 'l2tpv3' - since 2.1
-#
##
-{ 'union': 'NetClientOptions',
+{ 'union': 'Netdev',
+ 'base': { 'id': 'str', 'type': 'NetClientDriver' },
+ 'discriminator': 'type',
'data': {
'none': 'NetdevNoneOptions',
'nic': 'NetLegacyNicOptions',
@@ -2853,23 +2869,28 @@
'*vlan': 'int32',
'*id': 'str',
'*name':
[Xen-devel] [PATCH v8 12/16] qapi: Change Netdev into a flat union
This is a mostly-mechanical conversion that creates a new flat
union 'Netdev' QAPI type that covers all the branches of the
former 'NetClientOptions' simple union, where the branches are
now listed in a new 'NetClientDriver' enum rather than generated
from the simple union. The existence of a flat union has no
change to the command line syntax accepted for new code, and
will make it possible for a future patch to switch the QMP
command to parse a boxed union for no change to valid QMP; but
it does have some ripple effect on the C code when dealing with
the new types.
While making the conversion, note that the 'NetLegacy' type
remains unchanged: it applies only to legacy command line options,
and will not be ported to QMP, so it should remain a wrapper
around a simple union; to avoid confusion, the type named
'NetClientOptions' is now gone, and we introduce 'NetLegacyOptions'
in its place. Then, in the C code, we convert from NetLegacy to
Netdev as soon as possible, so that the bulk of the net stack
only has to deal with one QAPI type, not two. Note that since
the old legacy code always rejected 'hubport', we can just omit
that branch from the new 'NetLegacyOptions' simple union.
Based on an idea originally by Zoltán Kővágó <dirty.ice...@gmail.com>:
Message-Id:
<01a527fbf1a5de880091f98cf011616a78ad.1441627176.git.dirty.ice...@gmail.com>
although the sed script in that patch no longer applies due to
other changes in the tree since then, and I also did some manual
cleanups (such as fixing whitespace to keep checkpatch happy).
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
v8: rewrite commit message, claim authorship, rebase to latest
v7: rebase to latest master
v6: rebase to latest master
---
qapi-schema.json | 49 ++-
include/net/net.h| 4 +-
hw/arm/musicpal.c| 2 +-
hw/core/qdev-properties-system.c | 2 +-
hw/net/allwinner_emac.c | 2 +-
hw/net/cadence_gem.c | 2 +-
hw/net/dp8393x.c | 2 +-
hw/net/e1000.c | 2 +-
hw/net/e1000e.c | 2 +-
hw/net/eepro100.c| 2 +-
hw/net/etraxfs_eth.c | 2 +-
hw/net/fsl_etsec/etsec.c | 2 +-
hw/net/imx_fec.c | 2 +-
hw/net/lan9118.c | 2 +-
hw/net/lance.c | 2 +-
hw/net/mcf_fec.c | 2 +-
hw/net/milkymist-minimac2.c | 2 +-
hw/net/mipsnet.c | 2 +-
hw/net/ne2000-isa.c | 2 +-
hw/net/ne2000.c | 2 +-
hw/net/opencores_eth.c | 2 +-
hw/net/pcnet-pci.c | 2 +-
hw/net/rocker/rocker_fp.c| 2 +-
hw/net/rtl8139.c | 2 +-
hw/net/smc91c111.c | 2 +-
hw/net/spapr_llan.c | 2 +-
hw/net/stellaris_enet.c | 2 +-
hw/net/vhost_net.c | 20 +++---
hw/net/virtio-net.c | 10 +--
hw/net/vmxnet3.c | 2 +-
hw/net/xen_nic.c | 2 +-
hw/net/xgmac.c | 2 +-
hw/net/xilinx_axienet.c | 2 +-
hw/net/xilinx_ethlite.c | 2 +-
hw/usb/dev-network.c | 2 +-
monitor.c| 14 ++---
net/dump.c | 6 +-
net/filter.c | 2 +-
net/hub.c| 22 +++
net/l2tpv3.c | 6 +-
net/net.c| 133 +--
net/netmap.c | 4 +-
net/slirp.c | 6 +-
net/socket.c | 8 +--
net/tap-win32.c | 6 +-
net/tap.c| 24 +++
net/vde.c| 6 +-
net/vhost-user.c | 20 +++---
48 files changed, 230 insertions(+), 172 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index ba3bf14..e8a015f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2805,16 +2805,32 @@
'*queues':'int' } }
##
-# @NetClientOptions
+# @NetClientDriver
#
-# A discriminated record of network device traits.
+# Available netdev drivers.
+#
+# Since 2.7
+##
+{ 'enum': 'NetClientDriver',
+ 'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde', 'dump',
+'bridge', 'hubport', 'netmap', 'vhost-user' ] }
+
+##
+# @Netdev
+#
+# Captures the configuration of a network device.
+#
+# @id: identifier for monitor commands.
+#
+# @type: Specify the driver used for interpreting remaining arguments.
#
# Since 1.2
#
# 'l2tpv3' - since 2.1
-#
##
-{ 'union': 'NetClientOptions',
+{ 'union': 'Netdev',
+ 'base': { 'id': 'str', 'type': 'NetClientDriver' },
+ 'discriminator': 'type',
'data': {
'none': 'NetdevNoneOptions',
'nic': 'NetLegacyNicOptions',
@@ -2849,23 +2865,28 @@
'*vlan': 'int32',
'*id': 'str',
'*name':
[Xen-devel] [PATCH 05/17] block: Convert BB interface to byte-based discards
Change sector-based blk_discard(), blk_co_discard(), and
blk_aio_discard() to instead be byte-based *_pdiscard()
functions. NBD gets a lot simpler now that ignoring the
unaligned portion of a byte-based discard request is handled
under the hood by the block layer.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
include/sysemu/block-backend.h | 9 -
block/block-backend.c | 25 +++--
block/mirror.c | 5 +++--
hw/block/xen_disk.c| 7 ---
hw/ide/core.c | 6 --
hw/scsi/scsi-disk.c| 8
nbd/server.c | 19 +--
qemu-io-cmds.c | 3 +--
8 files changed, 36 insertions(+), 46 deletions(-)
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 2469a1c..778a994 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -138,15 +138,14 @@ BlockAIOCB *blk_aio_pwritev(BlockBackend *blk, int64_t
offset,
BlockCompletionFunc *cb, void *opaque);
BlockAIOCB *blk_aio_flush(BlockBackend *blk,
BlockCompletionFunc *cb, void *opaque);
-BlockAIOCB *blk_aio_discard(BlockBackend *blk,
-int64_t sector_num, int nb_sectors,
-BlockCompletionFunc *cb, void *opaque);
+BlockAIOCB *blk_aio_pdiscard(BlockBackend *blk, int64_t offset, int count,
+ BlockCompletionFunc *cb, void *opaque);
void blk_aio_cancel(BlockAIOCB *acb);
void blk_aio_cancel_async(BlockAIOCB *acb);
int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf);
BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
BlockCompletionFunc *cb, void *opaque);
-int blk_co_discard(BlockBackend *blk, int64_t sector_num, int nb_sectors);
+int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int count);
int blk_co_flush(BlockBackend *blk);
int blk_flush(BlockBackend *blk);
int blk_flush_all(void);
@@ -206,7 +205,7 @@ int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk,
int64_t offset,
int blk_write_compressed(BlockBackend *blk, int64_t sector_num,
const uint8_t *buf, int nb_sectors);
int blk_truncate(BlockBackend *blk, int64_t offset);
-int blk_discard(BlockBackend *blk, int64_t sector_num, int nb_sectors);
+int blk_pdiscard(BlockBackend *blk, int64_t offset, int count);
int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf,
int64_t pos, int size);
int blk_load_vmstate(BlockBackend *blk, uint8_t *buf, int64_t pos, int size);
diff --git a/block/block-backend.c b/block/block-backend.c
index 0a488f7..bf125c5 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1060,17 +1060,16 @@ BlockAIOCB *blk_aio_flush(BlockBackend *blk,
return bdrv_aio_flush(blk_bs(blk), cb, opaque);
}
-BlockAIOCB *blk_aio_discard(BlockBackend *blk,
-int64_t sector_num, int nb_sectors,
-BlockCompletionFunc *cb, void *opaque)
+BlockAIOCB *blk_aio_pdiscard(BlockBackend *blk,
+ int64_t offset, int count,
+ BlockCompletionFunc *cb, void *opaque)
{
-int ret = blk_check_request(blk, sector_num, nb_sectors);
+int ret = blk_check_byte_request(blk, offset, count);
if (ret < 0) {
return blk_abort_aio_request(blk, cb, opaque, ret);
}
-return bdrv_aio_pdiscard(blk_bs(blk), sector_num << BDRV_SECTOR_BITS,
- nb_sectors << BDRV_SECTOR_BITS, cb, opaque);
+return bdrv_aio_pdiscard(blk_bs(blk), offset, count, cb, opaque);
}
void blk_aio_cancel(BlockAIOCB *acb)
@@ -1102,15 +1101,14 @@ BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned
long int req, void *buf,
return bdrv_aio_ioctl(blk_bs(blk), req, buf, cb, opaque);
}
-int blk_co_discard(BlockBackend *blk, int64_t sector_num, int nb_sectors)
+int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int count)
{
-int ret = blk_check_request(blk, sector_num, nb_sectors);
+int ret = blk_check_byte_request(blk, offset, count);
if (ret < 0) {
return ret;
}
-return bdrv_co_pdiscard(blk_bs(blk), sector_num << BDRV_SECTOR_BITS,
-nb_sectors << BDRV_SECTOR_BITS);
+return bdrv_co_pdiscard(blk_bs(blk), offset, count);
}
int blk_co_flush(BlockBackend *blk)
@@ -1500,15 +1498,14 @@ int blk_truncate(BlockBackend *blk, int64_t offset)
return bdrv_truncate(blk_bs(blk), offset);
}
-int blk_discard(BlockBackend *blk, int64_t sector_num, int nb_sectors)
+int blk_pdiscard(BlockBackend *blk, int64_t offset, int count)
{
-int ret = blk_check_request(blk, sector_num, nb_sectors);
+int ret = blk_check_byte_request(blk, offset, count);
if (ret < 0) {
return ret;
}
-retur
Re: [Xen-devel] [Qemu-devel] [PATCH v7 11/15] qapi: Change Netdev into a flat union
On 06/16/2016 07:15 AM, Markus Armbruster wrote:
> Eric Blake <ebl...@redhat.com> writes:
>
>> From: Kővágó, Zoltán <dirty.ice...@gmail.com>
>>
>> Except qapi-schema.json, this patch was generated by:
>>
>> find . -name .git -prune -o -type f \! -name '*~' -print0 | \
>> xargs -0 sed -i \
>> -e 's/NetClientOptionsKind/NetClientDriver/g' \
>> -e 's/NET_CLIENT_OPTIONS_KIND_/NET_CLIENT_DRIVER_/g' \
>> -e 's/netdev->opts/netdev/g'
>
> Uh, this is prone to descend into build trees and edit random crap. I
> used
>
> $ sed -i -e ... `git-ls-tree -r HEAD | awk '$2 == "blob" { print $4 }'`
>
> to verify this commit. Differences noted inline.
>
> I additionally get:
>
> diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> index 692283f..825f0ba 100644
> --- a/hw/net/e1000e.c
> +++ b/hw/net/e1000e.c
> @@ -226,7 +226,7 @@ e1000e_set_link_status(NetClientState *nc)
>}
>
>static NetClientInfo net_e1000e_info = {
> -.type = NET_CLIENT_OPTIONS_KIND_NIC,
> +.type = NET_CLIENT_DRIVER_NIC,
>.size = sizeof(NICState),
>.can_receive = e1000e_nc_can_receive,
>.receive = e1000e_nc_receive,
>
> Rebase needed?
Unfortunately yes. This patch has been under a LOT of churn since it was
first written; it may be better to just redo it from scratch and claim
ownership myself, since it hardly resembles Zoltán's original submission
(but of course, give him credit for the idea).
>
>> index f8a500f..89a149b 100644
>> --- a/net/dump.c
>> +++ b/net/dump.c
>> @@ -172,7 +172,7 @@ static void dumpclient_cleanup(NetClientState *nc)
>> }
>>
>> static NetClientInfo net_dump_info = {
>> -.type = NET_CLIENT_OPTIONS_KIND_DUMP,
>> +.type = NET_CLIENT_DRIVER_DUMP,
>> .size = sizeof(DumpNetClient),
>> .receive = dumpclient_receive,
>> .receive_iov = dumpclient_receive_iov,
>> @@ -189,8 +189,8 @@ int net_init_dump(const Netdev *netdev, const char *name,
>> NetClientState *nc;
>> DumpNetClient *dnc;
>>
>> -assert(netdev->opts->type == NET_CLIENT_OPTIONS_KIND_DUMP);
>> -dump = netdev->opts->u.dump.data;
>> +assert(netdev->type == NET_CLIENT_DRIVER_DUMP);
>> +dump = >u.dump;
>
> sed turns this into
>
>dump = netdev->u.dump.data;
>
> Is this part of the manual changes? More of the same below.
>
The original sed script is so distant from the actual changes that it's
not worth mentioning the sed script in the commit message any more.
>>
>> assert(peer);
>>
>
> Another possible case of "rebase needed":
>
> diff --git a/net/filter.c b/net/filter.c
> index 8ac79f3..888fe6d 100644
> --- a/net/filter.c
> +++ b/net/filter.c
> @@ -201,7 +201,7 @@ static void netfilter_complete(UserCreatable *uc, Error
> **errp)
>}
>
>queues = qemu_find_net_clients_except(nf->netdev_id, ncs,
> - NET_CLIENT_OPTIONS_KIND_NIC,
> + NET_CLIENT_DRIVER_NIC,
> MAX_QUEUE_NUM);
Yep. Thanks for researching.
>>
>> static int net_client_init1(const void *object, int is_netdev, Error **errp)
>> {
>
> Multiple differences in this function. Manual?
Yes.
>> @@ -47,7 +47,7 @@ static void vhost_user_stop(int queues, NetClientState
>> *ncs[])
>> int i;
>>
>> for (i = 0; i < queues; i++) {
>> -assert (ncs[i]->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
>> +assert(ncs[i]->info->type == NET_CLIENT_DRIVER_VHOST_USER);
>
> Manual whitespace cleanup. Okay.
>
And necessary to shut up checkpatch. I really get to rewrite the commit
message to something better for v8, don't I.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] xen: Clean up includes
On 05/24/2016 09:27 AM, Peter Maydell wrote: > Clean up includes so that osdep.h is included first and headers > which it implies are not included manually. > > This commit was created with scripts/clean-includes. > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > --- > hw/usb/xen-usb.c | 5 + > include/hw/xen/xen.h | 1 - > 2 files changed, 1 insertion(+), 5 deletions(-) > Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v7 11/15] qapi: Change Netdev into a flat union
From: Kővágó, Zoltán <dirty.ice...@gmail.com>
Except qapi-schema.json, this patch was generated by:
find . -name .git -prune -o -type f \! -name '*~' -print0 | \
xargs -0 sed -i \
-e 's/NetClientOptionsKind/NetClientDriver/g' \
-e 's/NET_CLIENT_OPTIONS_KIND_/NET_CLIENT_DRIVER_/g' \
-e 's/netdev->opts/netdev/g'
Signed-off-by: Kővágó, Zoltán <dirty.ice...@gmail.com>
Message-Id:
<01a527fbf1a5de880091f98cf011616a78ad.1441627176.git.dirty.ice...@gmail.com>
Additional changes:
Rebase the patch on top of an earlier change from netdev->kind to
netdev->type, so that tweak is no longer needed here. Rebase to
latest master which enhanced multiqueue.
Rework so that NetdevLegacy doesn't pollute QMP command but is instead
copied piecewise into the new Netdev, which means that NetClientOptions
must still remain in qapi. Since legacy previously always rejected
'hubport', we can now make that explicit by having the two unions be
slightly different; but that means we must manually map between the
two structures.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
v7: rebase to latest master
v6: rebase to latest master
---
qapi-schema.json | 47 ++
include/net/net.h| 4 +-
hw/arm/musicpal.c| 2 +-
hw/core/qdev-properties-system.c | 2 +-
hw/net/allwinner_emac.c | 2 +-
hw/net/cadence_gem.c | 2 +-
hw/net/dp8393x.c | 2 +-
hw/net/e1000.c | 2 +-
hw/net/eepro100.c| 2 +-
hw/net/etraxfs_eth.c | 2 +-
hw/net/fsl_etsec/etsec.c | 2 +-
hw/net/imx_fec.c | 2 +-
hw/net/lan9118.c | 2 +-
hw/net/lance.c | 2 +-
hw/net/mcf_fec.c | 2 +-
hw/net/milkymist-minimac2.c | 2 +-
hw/net/mipsnet.c | 2 +-
hw/net/ne2000-isa.c | 2 +-
hw/net/ne2000.c | 2 +-
hw/net/opencores_eth.c | 2 +-
hw/net/pcnet-pci.c | 2 +-
hw/net/rocker/rocker_fp.c| 2 +-
hw/net/rtl8139.c | 2 +-
hw/net/smc91c111.c | 2 +-
hw/net/spapr_llan.c | 2 +-
hw/net/stellaris_enet.c | 2 +-
hw/net/vhost_net.c | 18 +++---
hw/net/virtio-net.c | 10 +--
hw/net/vmxnet3.c | 2 +-
hw/net/xen_nic.c | 2 +-
hw/net/xgmac.c | 2 +-
hw/net/xilinx_axienet.c | 2 +-
hw/net/xilinx_ethlite.c | 2 +-
hw/usb/dev-network.c | 2 +-
monitor.c| 14 ++---
net/dump.c | 6 +-
net/hub.c| 22 +++
net/l2tpv3.c | 6 +-
net/net.c| 133 +--
net/netmap.c | 4 +-
net/slirp.c | 6 +-
net/socket.c | 8 +--
net/tap-win32.c | 6 +-
net/tap.c| 24 +++
net/vde.c| 6 +-
net/vhost-user.c | 18 +++---
46 files changed, 225 insertions(+), 167 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 54634c4..4fee44b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2744,16 +2744,32 @@
'*queues':'int' } }
##
-# @NetClientOptions
+# @NetClientDriver
#
-# A discriminated record of network device traits.
+# Available netdev drivers.
+#
+# Since 2.7
+##
+{ 'enum': 'NetClientDriver',
+ 'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde', 'dump',
+'bridge', 'hubport', 'netmap', 'vhost-user' ] }
+
+##
+# @Netdev
+#
+# Captures the configuration of a network device.
+#
+# @id: identifier for monitor commands.
+#
+# @type: Specify the driver used for interpreting remaining arguments.
#
# Since 1.2
#
# 'l2tpv3' - since 2.1
-#
##
-{ 'union': 'NetClientOptions',
+{ 'union': 'Netdev',
+ 'base': { 'id': 'str', 'type': 'NetClientDriver' },
+ 'discriminator': 'type',
'data': {
'none': 'NetdevNoneOptions',
'nic': 'NetLegacyNicOptions',
@@ -2791,20 +2807,25 @@
'opts': 'NetClientOptions' } }
##
-# @Netdev
+# @NetClientOptions
#
-# Captures the configuration of a network device.
-#
-# @id: identifier for monitor commands.
-#
-# @opts: device type specific properties
+# Like Netdev, but for use only by the legacy command line options
#
# Since 1.2
##
-{ 'struct': 'Netdev',
+{ 'union': 'NetClientOptions',
'data': {
-'id': 'str',
-'opts': 'NetClientOptions' } }
+'none': 'NetdevNoneOptions',
+'nic': 'NetLegacyNicOptions',
+'user': 'NetdevUserOptions',
+'tap': 'NetdevTapOptions',
+'l2tpv3': 'NetdevL2TPv3Options',
+'socket': 'NetdevSocketOptions',
+'vde': 'NetdevVdeOptions',
+'dump':
[Xen-devel] [PATCH v7 08/19] xen_disk: Switch to byte-based aio block access
Sector-based blk_aio_readv() and blk_aio_writev() should die; switch
to byte-based blk_aio_preadv() and blk_aio_pwritev() instead.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
hw/block/xen_disk.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index d4ce380..064c116 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -554,9 +554,8 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
block_acct_start(blk_get_stats(blkdev->blk), >acct,
ioreq->v.size, BLOCK_ACCT_READ);
ioreq->aio_inflight++;
-blk_aio_readv(blkdev->blk, ioreq->start / BLOCK_SIZE,
- >v, ioreq->v.size / BLOCK_SIZE,
- qemu_aio_complete, ioreq);
+blk_aio_preadv(blkdev->blk, ioreq->start, >v, 0,
+ qemu_aio_complete, ioreq);
break;
case BLKIF_OP_WRITE:
case BLKIF_OP_FLUSH_DISKCACHE:
@@ -569,9 +568,8 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
ioreq->req.operation == BLKIF_OP_WRITE ?
BLOCK_ACCT_WRITE : BLOCK_ACCT_FLUSH);
ioreq->aio_inflight++;
-blk_aio_writev(blkdev->blk, ioreq->start / BLOCK_SIZE,
- >v, ioreq->v.size / BLOCK_SIZE,
- qemu_aio_complete, ioreq);
+blk_aio_pwritev(blkdev->blk, ioreq->start, >v, 0,
+qemu_aio_complete, ioreq);
break;
case BLKIF_OP_DISCARD:
{
--
2.5.5
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 09/20] xen_disk: Switch to byte-based aio block access
Sector-based blk_aio_readv() and blk_aio_writev() should die; switch
to byte-based blk_aio_preadv() and blk_aio_pwritev() instead.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
hw/block/xen_disk.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index d4ce380..064c116 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -554,9 +554,8 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
block_acct_start(blk_get_stats(blkdev->blk), >acct,
ioreq->v.size, BLOCK_ACCT_READ);
ioreq->aio_inflight++;
-blk_aio_readv(blkdev->blk, ioreq->start / BLOCK_SIZE,
- >v, ioreq->v.size / BLOCK_SIZE,
- qemu_aio_complete, ioreq);
+blk_aio_preadv(blkdev->blk, ioreq->start, >v, 0,
+ qemu_aio_complete, ioreq);
break;
case BLKIF_OP_WRITE:
case BLKIF_OP_FLUSH_DISKCACHE:
@@ -569,9 +568,8 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
ioreq->req.operation == BLKIF_OP_WRITE ?
BLOCK_ACCT_WRITE : BLOCK_ACCT_FLUSH);
ioreq->aio_inflight++;
-blk_aio_writev(blkdev->blk, ioreq->start / BLOCK_SIZE,
- >v, ioreq->v.size / BLOCK_SIZE,
- qemu_aio_complete, ioreq);
+blk_aio_pwritev(blkdev->blk, ioreq->start, >v, 0,
+qemu_aio_complete, ioreq);
break;
case BLKIF_OP_DISCARD:
{
--
2.5.5
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v6 09/16] qapi: Change Netdev into a flat union
From: Kővágó, Zoltán <dirty.ice...@gmail.com>
Except qapi-schema.json, this patch was generated by:
find . -name .git -prune -o -type f \! -name '*~' -print0 | \
xargs -0 sed -i \
-e 's/NetClientOptionsKind/NetClientDriver/g' \
-e 's/NET_CLIENT_OPTIONS_KIND_/NET_CLIENT_DRIVER_/g' \
-e 's/netdev->opts/netdev/g'
Signed-off-by: Kővágó, Zoltán <dirty.ice...@gmail.com>
Message-Id:
<01a527fbf1a5de880091f98cf011616a78ad.1441627176.git.dirty.ice...@gmail.com>
Additional changes:
Rebase the patch on top of an earlier change from netdev->kind to
netdev->type, so that tweak is no longer needed here. Rebase to
latest master which enhanced multiqueue.
Rework so that NetdevLegacy doesn't pollute QMP command but is instead
copied piecewise into the new Netdev, which means that NetClientOptions
must still remain in qapi. Since legacy previously always rejected
'hubport', we can now make that explicit by having the two unions be
slightly different; but that means we must manually map between the
two structures.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
v6: rebase to latest master
---
hw/arm/musicpal.c| 2 +-
hw/core/qdev-properties-system.c | 2 +-
hw/net/allwinner_emac.c | 2 +-
hw/net/cadence_gem.c | 2 +-
hw/net/dp8393x.c | 2 +-
hw/net/e1000.c | 2 +-
hw/net/eepro100.c| 2 +-
hw/net/etraxfs_eth.c | 2 +-
hw/net/fsl_etsec/etsec.c | 2 +-
hw/net/imx_fec.c | 2 +-
hw/net/lan9118.c | 2 +-
hw/net/lance.c | 2 +-
hw/net/mcf_fec.c | 2 +-
hw/net/milkymist-minimac2.c | 2 +-
hw/net/mipsnet.c | 2 +-
hw/net/ne2000-isa.c | 2 +-
hw/net/ne2000.c | 2 +-
hw/net/opencores_eth.c | 2 +-
hw/net/pcnet-pci.c | 2 +-
hw/net/rocker/rocker_fp.c| 2 +-
hw/net/rtl8139.c | 2 +-
hw/net/smc91c111.c | 2 +-
hw/net/spapr_llan.c | 2 +-
hw/net/stellaris_enet.c | 2 +-
hw/net/vhost_net.c | 16 ++---
hw/net/virtio-net.c | 10 +--
hw/net/vmxnet3.c | 2 +-
hw/net/xen_nic.c | 2 +-
hw/net/xgmac.c | 2 +-
hw/net/xilinx_axienet.c | 2 +-
hw/net/xilinx_ethlite.c | 2 +-
hw/usb/dev-network.c | 2 +-
include/net/net.h| 4 +-
monitor.c| 14 ++---
net/dump.c | 6 +-
net/hub.c| 22 +++
net/l2tpv3.c | 6 +-
net/net.c| 133 +--
net/netmap.c | 4 +-
net/slirp.c | 6 +-
net/socket.c | 8 +--
net/tap-win32.c | 6 +-
net/tap.c| 24 +++
net/vde.c| 6 +-
net/vhost-user.c | 18 +++---
qapi-schema.json | 57 +
46 files changed, 234 insertions(+), 166 deletions(-)
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index b534bb9..527b703 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -374,7 +374,7 @@ static void eth_cleanup(NetClientState *nc)
}
static NetClientInfo net_mv88w8618_info = {
-.type = NET_CLIENT_OPTIONS_KIND_NIC,
+.type = NET_CLIENT_DRIVER_NIC,
.size = sizeof(NICState),
.receive = eth_receive,
.cleanup = eth_cleanup,
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index ad3c428ff..959fdea 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -229,7 +229,7 @@ static void set_netdev(Object *obj, Visitor *v, const char
*name,
}
queues = qemu_find_net_clients_except(str, peers,
- NET_CLIENT_OPTIONS_KIND_NIC,
+ NET_CLIENT_DRIVER_NIC,
MAX_QUEUE_NUM);
if (queues == 0) {
err = -ENOENT;
diff --git a/hw/net/allwinner_emac.c b/hw/net/allwinner_emac.c
index 0407dee..4fdf824 100644
--- a/hw/net/allwinner_emac.c
+++ b/hw/net/allwinner_emac.c
@@ -422,7 +422,7 @@ static const MemoryRegionOps aw_emac_mem_ops = {
};
static NetClientInfo net_aw_emac_info = {
-.type = NET_CLIENT_OPTIONS_KIND_NIC,
+.type = NET_CLIENT_DRIVER_NIC,
.size = sizeof(NICState),
.can_receive = aw_emac_can_receive,
.receive = aw_emac_receive,
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 3639fc1..9fe7d19 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -1181,7 +1181,7 @@ static void gem_set_link(NetClientState *nc)
}
static NetClientInfo net_gem_info = {
-.type = NET_CL
Re: [Xen-devel] [Qemu-devel] Question about xen disk unplug support for ahci missed in qemu
On 10/16/2015 10:20 AM, Paul Durrant wrote: >> -Original Message- >>>>>>>>>>> On 10/13/2015 11:55 AM, Fabio Fantoni wrote: >>>>>>>>>>>> I added ahci disk support in libxl and using it for week seems Do we really need this many levels of quoting... >>>> that >>>>>> was >>>>>>>>>>>> ok, after a reply of Stefano Stabellini seems that xen disk >> unplug coupled with the result of poor mail clients that don't know how to properly reflow lines when quoting... >> Once you enable PV functionality, the IDE ports stop working; device >> reset disables the PV ring and goes back to IDE mode. No hard disk >> suddenly disappearing from the machine, no image corruption if the IDE >> device is written to before enabling PV, etc. >> > > That's not sufficient though. The IDE device must not be enumerated by the OS > and, in Windows at least, that enumeration occurs before the PV frontend has > started up. ...all before getting to the real new content of the message? It is not only okay to trim message, but actually encouraged on technical lists, so that you aren't wasting reader's time. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [edk2] EDK II & GPL - Re: OVMF BoF @ KVM Forum 2015
On 09/10/2015 08:14 PM, Kevin Davis wrote: >> >> >> On 10/09/2015 16:24, Kevin Davis wrote: >>> Further leading me to guess that any actual use of those >>> implementations could lead to you actually needing to hire a real >>> attorney and not one that you find on YouTube. >> >> The good thing is that attorneys have already figured it out. IBM figured >> out >> a few years ago how to work around Microsoft's patents, and that's how >> FAT32 (and more specifically long file names) are implemented in Linux. > > Ah. I wasn't in the room when they figured it out. And I've never seen > their written opinion. Is it documented somewhere? A bit of wikipedia reading turns up these indirect documentations of the solutions: http://arstechnica.com/information-technology/2009/07/vfat-linux-patch-could-circumvent-microsofts-patent-claims/ https://web.archive.org/web/20130131034455/http://www.desktoplinux.com/news/NS4980952387.html which in turn leads to this FAQ: https://web.archive.org/web/20121116185559/http://lkml.org/lkml/2009/6/26/314 So reading between the lines, IBM's opinion was that implementing a workaround that operates FAT in such a way that it never uses a common namespace was sufficient to avoid any legal questions about whether that code conflicts with a patent on a common namespace, sidestepping the longer question of any legal battle over the patent itself. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [libvirt] [PATCH libvirt] libxl: avoid freeing an uninitialised bitmap
On 06/19/2015 10:33 AM, Ian Campbell wrote: If vm-def-cputune.nvcpupin is 0 in libxlDomainSetVcpuAffinities (as seems to be the case on arm) then the VIR_FREE after cleanup: would be operating on an uninitialised pointer in map.map. Fix this by using libxl_bitmap_init and libxl_bitmap_dispose in the appropriate places (like VIR_FREE libxl_bitmap_dispose is also s/VIR_FREE/VIR_FREE,/ idempotent, so there is no double free on exit from the loop). libxl_bitmap_dispose is slightly preferable since it also sets map.size back to 0, avoiding a potential source of confusion. This fixes the crashes we've been seeing in the Xen automated tests on ARM. I had a glance at the handful of other users of libxl_bitmap and none of them looked to have a similar issue. Signed-off-by: Ian Campbell ian.campb...@citrix.com --- src/libxl/libxl_domain.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] qemu mainline regression with xen-unstable: unable to start QMP
[adding Markus, as author of the regression]
On 06/04/2015 03:59 PM, Don Slutz wrote:
On 06/04/15 11:04, Fabio Fantoni wrote:
Today after trying xen-unstable build (tested many hours) of some days
ago I tried update qemu to latest development version (from git master
commit 6fa6b312765f698dc81b2c30e7eeb9683804a05b) and seems that there is
a regression:
xl create /etc/xen/W7.cfg
Parsing config from /etc/xen/W7.cfg
libxl: error: libxl_qmp.c:287:qmp_handle_error_response: received an
error message from QMP server: QMP input object member 'id' is unexpected
libxl: error: libxl_qmp.c:715:libxl__qmp_initialize: Failed to connect
to QMP
This is caused by:
commit 65207c59d99f2260c5f1d3b9c491146616a522aa
Author: Markus Armbruster arm...@redhat.com
Date: Thu Mar 5 14:35:26 2015 +0100
monitor: Drop broken, unused asynchronous command interface
The patch:
From 1b0221078353870fe530e49de158cae205f9bce5 Mon Sep 17 00:00:00 2001
From: Don Slutz dsl...@verizon.com
Date: Thu, 4 Jun 2015 17:04:42 -0400
Subject: [PATCH 01/14] monitor: Allow Xen's (broken) usage of asynchronous
command interface.
commit 65207c59d99f2260c5f1d3b9c491146616a522aa
Author: Markus Armbruster arm...@redhat.com
Date: Thu Mar 5 14:35:26 2015 +0100
monitor: Drop broken, unused asynchronous command interface
Breaks Xen. Add a hack un unbreak it.
s/un /to /
Xen is only doing synchronous commands, but is including an id.
QMP also uses id, but apparently removes it up front before calling into
this function; so another fix would be having xen remove it up front.
Signed-off-by: Don Slutz dsl...@verizon.com
---
monitor.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/monitor.c b/monitor.c
index c7baa91..e9a0747 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4955,6 +4955,15 @@ static QDict *qmp_check_input_obj(QObject
*input_obj, Error **errp)
arguments, object);
return NULL;
}
+} else if (!strcmp(arg_name, id)) {
+/*
+ * Fixup Xen's usage. Just ignore the id. See point #5
+ * above. This was an attempt at an asynchronous
+ * command interface. However commit
+ * 65207c59d99f2260c5f1d3b9c491146616a522aa is
+ * wrong. Xen does not expect an error when it passes in
+ * id:1, so just continue to ignore it.
+ */
The comment is a bit verbose, particularly since 'id' is a
well-established usage pattern in QMP. Also, we don't need to call out
why it changed in the comment here, the commit message is sufficient for
that.
} else {
error_set(errp, QERR_QMP_EXTRA_MEMBER, arg_name);
return NULL;
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v6 1/6] Qemu-Xen-vTPM: Support for Xen stubdom vTPM command line options
On 05/04/2015 01:22 AM, Quan Xu wrote: Signed-off-by: Quan Xu quan...@intel.com --Changes in v6: -Remove stray insertion. --- configure| 14 ++ hmp.c| 2 ++ qapi-schema.json | 16 ++-- qemu-options.hx | 13 +++-- tpm.c| 7 ++- 5 files changed, 47 insertions(+), 5 deletions(-) Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 1/5] Qemu-Xen-vTPM: Support for Xen stubdom vTPM command line options
On 03/10/2015 06:14 AM, Quan Xu wrote:
--Changes in v4:
-qapi schema enhancement.
-remove no need code.
Patch history belongs...
Signed-off-by: Quan Xu quan...@intel.com
---
...here. It is useful to reviewers to know what changed since your last
submission, but not useful for the actual git history (where we will not
care if it took one revision or 20 on the mailing list before getting to
the one revision stored in git). More tips:
http://wiki.qemu.org/Contribute/SubmitAPatch
configure| 14 ++
hmp.c| 2 ++
qapi-schema.json | 18 --
qemu-options.hx | 13 +++--
tpm.c| 7 ++-
5 files changed, 49 insertions(+), 5 deletions(-)
+++ b/qapi-schema.json
@@ -2854,9 +2854,11 @@
#
# @passthrough: TPM passthrough type
#
+# @xenstubdoms: TPM xenstubdoms type (since 2.3)
+#
# Since: 1.5
##
-{ 'enum': 'TpmType', 'data': [ 'passthrough' ] }
+{ 'enum': 'TpmType', 'data': [ 'passthrough', 'xenstubdoms' ] }
##
# @query-tpm-types:
@@ -2884,6 +2886,16 @@
{ 'type': 'TPMPassthroughOptions', 'data': { '*path' : 'str',
'*cancel-path' : 'str'} }
+# @TPMXenstubdomsOptions:
Missing a '##' lead-in line.
+#
+# Information about the TPM xenstubdoms type
+#
+# Since: 2.3
+##
+{ 'type': 'TPMXenstubdomsOptions', 'data': { } }
+#
+##
+
##
We don't usually have trailing '#' or '##' lines after a type declaration.
# @TpmTypeOptions:
#
@@ -2894,7 +2906,9 @@
# Since: 1.5
##
{ 'union': 'TpmTypeOptions',
- 'data': { 'passthrough' : 'TPMPassthroughOptions' } }
+ 'data': { 'passthrough' : 'TPMPassthroughOptions',
+'xenstubdoms' : 'TPMXenstubdomsOptions' } }
+##
We are already in soft freeze for 2.3, and this is a new feature. Is it
still going to make it, or should you adjust this patch to say 'since 2.4'?
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [libvirt] [PATCH] tests: fix xlconfigtest build failure
On 01/20/2015 09:09 PM, Jim Fehlig wrote: When libvirt is configured --without-xen, building the xlconfigtest fails with CCLD xlconfigtest /usr/lib/gcc/x86_64-linux-gnu/4.7/../../../x86_64-linux-gnu/crt1.o In function `_start': (.text+0x20): undefined reference to `main' collect2: error: ld returned 1 exit status Introduced in commit 4ed5fb91 by too much copy and paste from xmconfigtest. Signed-off-by: Jim Fehlig jfeh...@suse.com --- tests/Makefile.am| 17 - tests/xlconfigtest.c | 4 ++-- 2 files changed, 14 insertions(+), 7 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [libvirt] [PATCH V4 1/3] Introduce support for parsing/formatting Xen xl config format
On 01/13/2015 04:47 PM, Jim Fehlig wrote: +++ b/src/Makefile.am @@ -1005,6 +1005,10 @@ XENCONFIG_SOURCES = \ xenconfig/xen_common.c xenconfig/xen_common.h \ xenconfig/xen_sxpr.c xenconfig/xen_sxpr.h \ xenconfig/xen_xm.c xenconfig/xen_xm.h +if WITH_LIBXL +XENCONFIG_SOURCES += \ + xenconfig/xen_xl.c xenconfig/xen_xl.h +endif WITH_LIBXL Missing an EXTRA_DIST listing to ensure these two files are part of a tarball even when configure does not build libxl sources (that is, make sure 'make distcheck' will not fail if configured on a machine without libxl support). Several of the tests fail on the old distro I'm testing on, which causes 'make distcheck' to fail. But I assumed I could simulate it on a newer distro with 'configure --without-libxl', yet 'make distcheck' succeeds and the files are included in the tarball. Are you seeing a failure on RHEL5? I haven't yet tested a 'make distcheck' on that particular VM of mine; I'll fire one off and see what happens. It might be simpler to just to check that if 'make dist' is run when configured --without-libxl, then it still includes both files in the tarball. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [libvirt] [PATCH V4 2/3] tests: Tests for the xen-xl parser
On 01/13/2015 08:53 AM, Jim Fehlig wrote: From: Kiarie Kahurani davidkiar...@gmail.com add tests for the xen_xl config parser Signed-off-by: Kiarie Kahurani davidkiar...@gmail.com Signed-off-by: Jim Fehlig jfeh...@suse.com --- V4: Only build xlconfigtest when libxl is available. @@ -227,6 +228,11 @@ if WITH_XEN test_programs += xml2sexprtest sexpr2xmltest \ xmconfigtest xencapstest statstest reconnect endif WITH_XEN + +if WITH_LIBXL +test_programs += xlconfigtest +endif WITH_LIBXL Nice. + +DO_TEST(new-disk, 3); +//DO_TEST(spice, 3); + Do we still need this comment? ACK with that resolved. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [libvirt] [PATCH V4 3/3] libxl: Add support for parsing/formating Xen XL config
On 01/13/2015 08:53 AM, Jim Fehlig wrote:
From: Kiarie Kahurani davidkiar...@gmail.com
Now that xenconfig supports parsing and formatting Xen's
XL config format, integrate it into the libxl driver's
connectDomainXML{From,To}Native functions.
Signed-off-by: Kiarie Kahurani davidkiar...@gmail.com
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
V4:
Note support for new 'xen-xl' format in virsh man page.
src/libxl/libxl_driver.c | 32
tools/virsh.pod | 8
2 files changed, 28 insertions(+), 12 deletions(-)
Thanks for the man page docs.
[Someday, I'd like to add an API or modify an existing capability API
that returns XML to make it possible to query at runtime what formats
are supported by the format-{to,from}-native calls - but that doesn't
have to hold up this series]
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [libvirt] [PATCH V4 0/3] Parser for xen-xl config format
On 01/13/2015 08:53 AM, Jim Fehlig wrote: It's been a long, twisting road to V4 of the Xen xl parser. V3 [1] was based on a flex-based parser that was copied from the Xen project and proved to be a bit challenging to integrate properly with autotools. But as it turns out, Xen provides an interface to the parser via libxlutil. I hadn't realized this interface was available for external consumption since the corresponding header file was never installed. Patch sent to xen-devel [2] to install the header, but in the meantime need to declare gthe imported libxlutil functions as extern (see patch 1). V4 uses libxlutil, which has simplified the series quite a bit and eliminates the potential of the copied flex parser diverging from the canonical source in xen.git. [1] https://www.redhat.com/archives/libvir-list/2014-December/msg00765.html [2] http://lists.xen.org/archives/html/xen-devel/2015-01/msg00690.html Yay - this series compiled on RHEL 5, with no extra efforts on my part. I'll review the individual patches as well, but we're already looking better. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [libvirt] [PATCH V4 1/3] Introduce support for parsing/formatting Xen xl config format
On 01/13/2015 08:53 AM, Jim Fehlig wrote:
Introduce a parser/formatter for the xl config format. Since the
deprecation of xm/xend, the VM config file format has diverged as
new features are added to libxl. This patch adds support for parsing
and formating the xl config format. It supports the existing xm config
format, plus adds support for spice graphics and xl disk config syntax.
xl disk config is parsed with the help of xlu_disk_parse() from
libxlutil, libxl's utility library. Although the library exists
in all Xen versions supported by the libxl virt driver, only
recently has the corresponding header file been included. A check
for the header is done in configure.ac. If not found, xlu_disk_parse()
is declared externally.
Signed-off-by: Kiarie Kahurani davidkiar...@gmail.com
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
+++ b/src/Makefile.am
@@ -1005,6 +1005,10 @@ XENCONFIG_SOURCES =
\
xenconfig/xen_common.c xenconfig/xen_common.h \
xenconfig/xen_sxpr.c xenconfig/xen_sxpr.h \
xenconfig/xen_xm.c xenconfig/xen_xm.h
+if WITH_LIBXL
+XENCONFIG_SOURCES += \
+ xenconfig/xen_xl.c xenconfig/xen_xl.h
+endif WITH_LIBXL
Missing an EXTRA_DIST listing to ensure these two files are part of a
tarball even when configure does not build libxl sources (that is, make
sure 'make distcheck' will not fail if configured on a machine without
libxl support).
+xenParseXLDisk(virConfPtr conf, virDomainDefPtr def)
+while (list) {
+const char *disk_spec = list-str;
+
+if ((list-type != VIR_CONF_STRING) || (list-str == NULL))
Over-parenthesized.
+goto skipdisk;
+
+libxl_device_disk_init(libxldisk);
+
+if (xlu_disk_parse(xluconf, 1, disk_spec, libxldisk))
+goto fail;
+
+if (!(disk = virDomainDiskDefNew()))
+goto fail;
+
+if (VIR_STRDUP(disk-dst, libxldisk-vdev) 0)
+goto fail;
+
+if (virDomainDiskSetSource(disk, libxldisk-pdev_path) 0)
+goto fail;
+
+disk-src-readonly = libxldisk-readwrite ? 0 : 1;
Isn't disk-src-readonly a bool? In which case, this should be:
disk-src-readonly = !libxldisk-readwrite;
for correct typing.
I'd wait for John to confirm that Coverity is happy, but ACK if you fix
the spots I pointed out.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [libvirt] [PATCH 00/12] Replace Xen xl parsing/formatting impl
On 01/12/2015 08:06 AM, Ian Campbell wrote: On Fri, 2015-01-09 at 22:03 -0700, Jim Fehlig wrote: The first attempt to implement support for parsing/formatting Xen's xl disk config format copied Xen's flex-based parser into libvirt, which has proved to be challenging in the context of autotools. But as it turns out, Xen provides an interface to the parser via libxlutil. This series reverts the first attempt, along with subsequent attempts to fix it, and replaces it with an implementation based on libxlutil. The first nine patches revert the original implementation and subsequent fixes. Patch 10 provides an implemenation based on libxlutil. Patches 11 and 12 are basically unchanged from patches 3 and 4 in the first attempt. One upshot of using libxlutil instead of copying the flex source is removing the potential for source divergence. Thanks for doing this, looks good to me, FWIW. Is the presence/absence of xen-xl support exposed via virsh anywhere? If so then I can arrange for my Xen osstest patches for libvirt testing to use xen-xl when available but still fallback to xen-xm. I've had a look in virsh capabilities and virsh help domxml-from-native but not seeing xen-xm, so assuming xen-xl won't magically appear in any of those places either. I'm not sure if 'virsh capabilities' can show it, but it does sound like a nice place to enhance if possible. Also, if 'virsh --version=long' doesn't state whether libxl support was compiled in, it should be patched to do so; although that only shows what the client side supports (and not necessarily what the remote server side supports). -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [libvirt] [PATCH 09/12] Revert src/xenconfig: Xen-xl parser
On 01/12/2015 07:51 AM, John Ferlan wrote:
On 01/10/2015 12:03 AM, Jim Fehlig wrote:
This reverts commit 2c78051a14acfb7aba078d569b1632dfe0ca0853.
Conflicts:
src/Makefile.am
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
.gitignore| 1 -
cfg.mk| 3 +-
configure.ac | 1 -
po/POTFILES.in| 1 -
src/Makefile.am | 25 +--
src/libvirt_xenconfig.syms| 4 -
src/xenconfig/xen_common.c| 3 +-
src/xenconfig/xen_xl.c| 499
--
src/xenconfig/xen_xl.h| 33 ---
src/xenconfig/xen_xl_disk.l | 256 --
src/xenconfig/xen_xl_disk_i.h | 39
11 files changed, 4 insertions(+), 861 deletions(-)
OK - so reverting is fine; however, xen_xl_disk.{c,h} still exist...
Simple enough solution, but they will show up in someone's git status
output since they are also removed from .gitignore.
It's a transient issue - someone that only checked out git at v1.2.11
won't see the generated files; the few of us that do incremental work
can modify .git/info/exclude manually to ignore leftover generated
files. But if you also want to explicitly ignore the generated files in
.gitignore, go for it.
ACK 1-9, and I'm liking the initial work of 10-12 other than the
Coverity issues that it introduces.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [libvirt] [PATCH 10/12] Introduce support for parsing/formatting Xen xl config format
On 01/09/2015 10:03 PM, Jim Fehlig wrote: Introduce a parser/formatter for the xl config format. Since the deprecation of xm/xend, the VM config file format has diverged as new features are added to libxl. This patch adds support for parsing and formating the xl config format. It supports the existing xm config format, plus adds support for spice graphics and xl disk config syntax. xl disk config is parsed with the help of xlu_disk_parse() from libxlutil, libxl's utility library. Although the library exists in all Xen versions supported by the libxl virt driver, only recently has the corresponding header file been included. A check for the header is done in configure.ac. If not found, xlu_disk_parse() is declared externally. Signed-off-by: Kiarie Kahurani davidkiar...@gmail.com Signed-off-by: Jim Fehlig jfeh...@suse.com --- configure.ac | 3 + po/POTFILES.in | 1 + src/Makefile.am| 4 +- src/libvirt_xenconfig.syms | 4 + src/xenconfig/xen_common.c | 3 +- src/xenconfig/xen_xl.c | 492 + src/xenconfig/xen_xl.h | 33 +++ 7 files changed, 538 insertions(+), 2 deletions(-) This patch fails to build on RHEL 5: http://fpaste.org/168738/84783142/ There, the version of xen is so old that there is no support for newer xenlight interaction; we still want the old xen support code to compile, but you should probably consider making things conditional so that xen_xl.c is attempted only when new-enough xenlight support exists (the same way that src/libxl is avoided when only older xen is present). It may be as simple as making the file conditional on HAVE_LIBXL, although I haven't yet tried that. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [libvirt] [PATCH RFC] libxl: fix paths in capability string
On 01/06/2015 09:12 AM, Wei Liu wrote: Currently libxl driver hardcodes some paths in its capability string, which might not be the correct paths. This patch introduces --with-libxl-prefix, so that user can specify the prefix used to build Xen tools. The default value is /usr/local which is the default --prefix for Xen tools. Why can't you just make '--with-libxl=/path/to/alt' work? That is, --with-libxl=yes uses a default (which matches the prefix where LIBVIRT will be installed [1]), --with-libxl=no turns it off, and specifying a path says to use that path. Then you don't need to add a new option. [1] The assumption generally is that whoever is building libvirt has also built libxl to be installed in the same location - which is a nicer default than hard-coding a /usr/local default that libxl uses in isolation. Of course, libvirt also defaults to /usr/local in isolation, so if you don't specify anything at all, then ./configure will see that libvirt is going into /usr/local and will therefore default to looking for libxl also in /usr/local; but for a distro packager, it is nicer to have the mere specification of --prefix switch all other relative defaults to play nicely with everything else in the distro. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [v3 1/5] Qemu-Xen-vTPM: Support for Xen stubdom vTPM command line options
On 12/30/2014 04:02 PM, Quan Xu wrote: Signed-off-by: Quan Xu quan...@intel.com This message was missing an In-Reply-To header tying it to the 0/5 cover letter, making it show up as an independent thread. Please see if you can fix that for the next submission. --- configure| 14 ++ hmp.c| 7 +++ qapi-schema.json | 19 --- qemu-options.hx | 13 +++-- tpm.c| 7 ++- 5 files changed, 54 insertions(+), 6 deletions(-) +++ b/qapi-schema.json @@ -2854,9 +2854,10 @@ # # @passthrough: TPM passthrough type # -# Since: 1.5 +# @xenstubdoms: TPM xenstubdoms type (since 2.3)## Since 1.5 Missing newlines. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [v2 1/4] Qemu-Xen-vTPM: Support for Xen stubdom vTPM command line options
On 11/25/2014 06:51 AM, Xu, Quan wrote: Also, this message was not threaded properly; it appeared as a top-level thread along with three other threads for its siblings, instead of all four patches being in-reply-to the 0/4 cover letter. Thanks Eric. Should I: V2 is version number, 1. $git format-patch --subject-prefix=v2 -ns --cover-letter master Then, edit -cover-letter.patch Rather than '--subject-prefix=v2', I prefer using '-v2'. The important part is that you send the mails with 'git send-email' instead of doing it by hand. In fact, you can do 'git send-email --annotate -v2 --cover-letter master' and skip the format-patch altogether, if you are okay saving your cover letter edits to the point where you are sending the mails. 2. $git format-patch --subject-prefix=v2 -4 Then, commit these 4 patch and -cover-letter.patch I'm not sure what you meant by commit. You aren't adding the *.patch files to a repository, but sending them as email. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
