Re: [Qemu-devel] [PATCH 1/1] block: fix inability to start VM with native AIO
On Tue, Dec 22, 2015 at 09:59:46AM +0300, Denis V. Lunev wrote: > error: Failed to start domain rhel7 > error: internal error: process exited while connecting to monitor: > 2015-12-22T06:55:18.812637Z qemu-system-x86_64: > -drive file=/var/lib/libvirt/images/rhel7.qcow2,if=none, > id=drive-scsi0-0-0-0,format=qcow2,cache=none,aio=native: > aio=native was specified, but it requires cache.direct=on, > which was not specified. > > cache=none option was specified as seen above while the VM is unable to > start. The patch properly passed BDRV_O_NOCACHE to underlying layer. > > The problem is revealed with > commit d657c0c289e944fc22289f5c318f48da87d79dcb > Author: Kevin Wolf > Date: Tue Dec 15 11:35:36 2015 +0100 > > raw-posix: Make aio=native option binding > > Signed-off-by: Denis V. Lunev > CC: Kevin Wolf > --- > block.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/block.c b/block.c > index 411edbf..fe0fbbc 100644 > --- a/block.c > +++ b/block.c > @@ -990,6 +990,7 @@ static int bdrv_open_common(BlockDriverState *bs, > BdrvChild *file, > bs->opaque = g_malloc0(drv->instance_size); > > /* Apply cache mode options */ > +update_flags_from_options(&open_flags, opts); > update_flags_from_options(&bs->open_flags, opts); > bdrv_set_enable_write_cache(bs, bs->open_flags & BDRV_O_CACHE_WB); I tried to review this patch and failed to understand block.c flags handling. Perhaps there is a larger problem here because I see: 1. .bdrv_open() and friends take an int flags argument in addition to the already open bs node, which has bs->open_flags. Some of the code that manipulates the flags argument in block.c updates bs->open_flags to keep them in sync. Some code does not (e.g. BDRV_O_NO_BACKING). Is this a bug? Should int flags be removed in favor of just bs->open_flags? 2. The bdrv_open_common() open_flags local variable contains different values from bs->open_flags (i.e. BDRV_O_PROTOCOL). I'm not sure whether this is intentional or a bug. Your patch syncs them but I wonder if it would be cleaner to remove the open_flags local (avoiding similar problems in the future)? 3. block/qcow2.c stashes .bdrv_open(flags) in s->flags. I'm not sure if bs->open_flags can be used instead of s->flags. That would be simpler. Maybe someone can explain how this is all supposed to work. Stefan signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 1/1] block: fix inability to start VM with native AIO
On Tue, Dec 22, 2015 at 09:59:46AM +0300, Denis V. Lunev wrote: > error: Failed to start domain rhel7 > error: internal error: process exited while connecting to monitor: > 2015-12-22T06:55:18.812637Z qemu-system-x86_64: > -drive file=/var/lib/libvirt/images/rhel7.qcow2,if=none, > id=drive-scsi0-0-0-0,format=qcow2,cache=none,aio=native: > aio=native was specified, but it requires cache.direct=on, > which was not specified. > > cache=none option was specified as seen above while the VM is unable to > start. The patch properly passed BDRV_O_NOCACHE to underlying layer. > > The problem is revealed with > commit d657c0c289e944fc22289f5c318f48da87d79dcb > Author: Kevin Wolf > Date: Tue Dec 15 11:35:36 2015 +0100 > > raw-posix: Make aio=native option binding > > Signed-off-by: Denis V. Lunev > CC: Kevin Wolf > --- > block.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/block.c b/block.c > index 411edbf..fe0fbbc 100644 > --- a/block.c > +++ b/block.c > @@ -990,6 +990,7 @@ static int bdrv_open_common(BlockDriverState *bs, > BdrvChild *file, > bs->opaque = g_malloc0(drv->instance_size); > > /* Apply cache mode options */ > +update_flags_from_options(&open_flags, opts); > update_flags_from_options(&bs->open_flags, opts); > bdrv_set_enable_write_cache(bs, bs->open_flags & BDRV_O_CACHE_WB); By the way, my questions do not prevent your patch from being merged. I just wanted to raise them for Kevin because I'm concerned there are more bugs in this area. signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH RFC 0/5] generic image locking and crash recovery
On Wed, 12/23 10:46, Denis V. Lunev wrote: > This series of patches is aimed to prevent usage of image > file by different qemu instances. In case we are the first > instance, and option lock is lockfile, - we lock the image file, > and if check option is on, we check the file and fix it if > nessecary. If one of this two ops fails - the image is closed > with the error. > > Patchset is not polished at all! Sent for a discussion as an alternative > approach. I like this approach. The first two patches match what I was thinking of. Patch 5 is okay, the unclean flag reflects HEADER_INUSE_MAGIC in parallels header; unfortunately patch 4 is wrong because qcow2 lacks a counterpart flag in the format, and the patch only modified an in memory variable. we have to add this as a compatible_features bit in order to support this operation. Didn't review very closely because at least one patch doesn't seem to compile. :) Fam > > Signed-off-by: Olga Krishtal > Signed-off-by: Denis V. Lunev > CC: Kevin Wolf > CC: Max Reitz > CC: Eric Blake > CC: Fam Zheng > > Olga Krishtal (5): > block: added lock image option and callback > block: implemented bdrv_lock_image for raw file > block: added check image option and callback bdrv_is_opened_unclean > qcow2: implemented bdrv_is_opened_unclean > block/paralels: added paralles implementation for > bdrv_is_opened_unclean > > block.c | 73 > +++ > block/parallels.c | 7 - > block/qcow2.c | 11 ++- > block/qcow2.h | 1 + > block/raw-posix.c | 15 ++ > block/raw-win32.c | 19 > include/block/block.h | 2 ++ > include/block/block_int.h | 2 ++ > qapi/block-core.json | 9 ++ > 9 files changed, 137 insertions(+), 2 deletions(-) > > -- > 2.1.4 >
Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr/pci: populate PCI DT in reverse order
David Gibson writes: > On Thu, Dec 17, 2015 at 09:43:29AM +0100, Greg Kurz wrote: >> On Thu, 3 Dec 2015 15:53:17 +0100 >> Greg Kurz wrote: >> >> > On Tue, 1 Dec 2015 22:48:38 +0100 >> > Thomas Huth wrote: >> > >> > > On 30/11/15 11:45, Greg Kurz wrote: >> > > > Since commit 1d2d974244c6 "spapr_pci: enumerate and add PCI device >> > > > tree", QEMU >> > > > populates the PCI device tree in the opposite order compared to SLOF. >> > > > >> > > > Before 1d2d974244c6: >> > > > >> > > > Populating /pci@8002000 >> > > > 00 (D) : 1af4 1000virtio [ net ] >> > > > 00 0800 (D) : 1af4 1001virtio [ block ] >> > > > 00 1000 (D) : 1af4 1009virtio [ network ] >> > > > Populating /pci@8002000/unknown-legacy-device@2 >> > > > >> > > > >> > > > 7e5294b8 : /pci@8002000 >> > > > 7e52b998 : |-- ethernet@0 >> > > > 7e52c0c8 : |-- scsi@1 >> > > > 7e52c7e8 : +-- unknown-legacy-device@2 ok >> > > > >> > > > Since 1d2d974244c6: >> > > > >> > > > Populating /pci@8002000 >> > > > 00 1000 (D) : 1af4 1009virtio [ network ] >> > > > Populating /pci@8002000/unknown-legacy-device@2 >> > > > 00 0800 (D) : 1af4 1001virtio [ block ] >> > > > 00 (D) : 1af4 1000virtio [ net ] >> > > > >> > > > >> > > > 7e5e8118 : /pci@8002000 >> > > > 7e5ea6a0 : |-- unknown-legacy-device@2 >> > > > 7e5eadb8 : |-- scsi@1 >> > > > 7e5eb4d8 : +-- ethernet@0 ok >> > > > >> > > > This behaviour change is not actually a bug since no assumptions >> > > > should be >> > > > made on DT ordering. But it has no real justification either, other >> > > > than >> > > > being the consequence of the way fdt_add_subnode() inserts new elements >> > > > to the front of the FDT rather than adding them to the tail. >> > > > >> > > > This patch reverts to the historical SLOF ordering by walking PCI >> > > > devices in >> > > > reverse order. >> > > >> > > I've applied your patch here locally, and indeed, the device tree looks >> > > nicer to me, too, when the nodes are listed in ascending order. >> > > >> > > Tested-by: Thomas Huth >> > > >> > > >> > >> >> Ping ? > > Sorry I didn't reply. > > I'm still dubious about this. It seems like a fair bit of effort to > restore a behaviour that the client isn't supposed to be relying on > anyway. > > Plus, the version with the changed order is already released, so > applying this will mean a second behaviour change. The behaviour change was not intentional by me, so I would vote for restoring the old order. Reviewed-by: Nikunj A Dadhania Regards Nikunj
Re: [Qemu-devel] [PATCH v2 06/14] qapi: Add qstring_append_format()
On Mon, 12/21 17:31, Eric Blake wrote: > Back in commit 764c1ca (Nov 2009), we added qstring_append_int(). > However, it did not see any use until commit 190c882 (Jan 2015). > Furthermore, it has a rather limited use case - to print anything > else, callers still have to format into a temporary buffer, unless > we want to introduce an explosion of new qstring_append_* methods > for each useful type to print. > > A much better approach is to add a wrapper that merges printf > behavior onto qstring_append, via the new qstring_append_format() > (and its vararg counterpart). In fact, with that in place, we > no longer need qstring_append_int(). > > Other immediate uses for the new function include simplifying > two existing clients of qstring_append() on a just-formatted > buffer, and the fact that we can take advantage of printf width > manipulations for more efficient indentation. > > Signed-off-by: Eric Blake Reviewed-by: Fam Zheng
Re: [Qemu-devel] Minutes from the "Stuttgart block Gipfele"
On Fri, Dec 18, 2015 at 02:15:38PM +0100, Markus Armbruster wrote: > What should happen when the user asks for a mutation at a place where we > have implicit filter(s)? Please suspend your disbelief for a second: In principle it's simplest not having implicit filters. The client needs to set up throttling nodes or the backup filter explicitly. Okay, now it's time to tear this apart: For backwards compatibility it's necessary to support throttling, copy-on-read, backup notifier, etc. It may be possible to tag implicit filter nodes so that mutation operations that wouldn't be possible today are rejected. The client must use the explicit syntax to do mutations on implicit filters. This is easier said than done, I'm not sure it can be implemented cleanly. Another problem is that the backup block job and other operations that require a single command today could require sequences of low-level setup commands to create filter nodes. The QMP client would need to first create a write notifier filter and then start the backup block job with the write notifier node name. It's clumsy. Stefan signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH RFC 0/5] generic image locking and crash recovery
On 12/23/2015 11:09 AM, Fam Zheng wrote: On Wed, 12/23 10:46, Denis V. Lunev wrote: This series of patches is aimed to prevent usage of image file by different qemu instances. In case we are the first instance, and option lock is lockfile, - we lock the image file, and if check option is on, we check the file and fix it if nessecary. If one of this two ops fails - the image is closed with the error. Patchset is not polished at all! Sent for a discussion as an alternative approach. I like this approach. The first two patches match what I was thinking of. Patch 5 is okay, the unclean flag reflects HEADER_INUSE_MAGIC in parallels header; unfortunately patch 4 is wrong because qcow2 lacks a counterpart flag in the format, and the patch only modified an in memory variable. we have to add this as a compatible_features bit in order to support this operation. yep :) this could be done, no problem. Didn't review very closely because at least one patch doesn't seem to compile. :) Fam which compile error do you have? I have double checked that it is compiled OK on commit 5dc42c186d63b7b338594fc071cf290805dcc5a5 Merge: c595b21 7236975 Author: Peter Maydell Date: Tue Dec 22 14:21:42 2015 + Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging # gpg: Signature made Tue 22 Dec 2015 08:52:55 GMT using RSA key ID 81AB73C8 # gpg: Good signature from "Stefan Hajnoczi " # gpg: aka "Stefan Hajnoczi " * remotes/stefanha/tags/block-pull-request: sdhci: add optional quirk property to disable card insertion/removal interrupts sdhci: don't raise a command index error for an unexpected response sd: sdhci: Delete over-zealous power check scripts/gdb: Fix a python exception in mtree.py parallels: add format spec block/mirror: replace IOV_MAX with blk_get_max_iov() block: replace IOV_MAX with BlockLimits.max_iov block-backend: add blk_get_max_iov() block: add BlockLimits.max_iov field virtio-blk: trivial code optimization Signed-off-by: Peter Maydell Den
Re: [Qemu-devel] [PATCH v2 1/2] igd-passthrough-i440FX: convert to realize()
Hi mst,
Ping
Or do I need to cc this to qemu-trivial?
On 12/21/2015 07:00 PM, Cao jin wrote:
Signed-off-by: Cao jin
---
hw/pci-host/piix.c | 32 +---
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 715208b..bc0fc59 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -761,7 +761,7 @@ static const IGDHostInfo igd_host_bridge_infos[] = {
{0xa8, 4}, /* SNB: base of GTT stolen memory */
};
-static int host_pci_config_read(int pos, int len, uint32_t val)
+static void host_pci_config_read(int pos, int len, uint32_t val, Error **errp)
{
char path[PATH_MAX];
int config_fd;
@@ -769,50 +769,52 @@ static int host_pci_config_read(int pos, int len,
uint32_t val)
/* Access real host bridge. */
int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s",
0, 0, 0, 0, "config");
-int ret = 0;
if (rc >= size || rc < 0) {
-return -ENODEV;
+error_setg_errno(errp, errno, "snprintf err");
+return;
}
config_fd = open(path, O_RDWR);
if (config_fd < 0) {
-return -ENODEV;
+error_setg_errno(errp, errno, "open err");
+return;
}
if (lseek(config_fd, pos, SEEK_SET) != pos) {
-ret = -errno;
+error_setg_errno(errp, errno, "lseek err");
goto out;
}
+
do {
rc = read(config_fd, (uint8_t *)&val, len);
} while (rc < 0 && (errno == EINTR || errno == EAGAIN));
if (rc != len) {
-ret = -errno;
+error_setg_errno(errp, errno, "read err");
}
+
out:
close(config_fd);
-return ret;
}
-static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
+static void igd_pt_i440fx_realize(struct PCIDevice *pci_dev, Error **errp)
{
uint32_t val = 0;
-int rc, i, num;
+int i, num;
int pos, len;
+Error *local_err = NULL;
num = ARRAY_SIZE(igd_host_bridge_infos);
for (i = 0; i < num; i++) {
pos = igd_host_bridge_infos[i].offset;
len = igd_host_bridge_infos[i].len;
-rc = host_pci_config_read(pos, len, val);
-if (rc) {
-return -ENODEV;
+host_pci_config_read(pos, len, val, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
}
pci_default_write_config(pci_dev, pos, val, len);
}
-
-return 0;
}
static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
@@ -820,7 +822,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass
*klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-k->init = igd_pt_i440fx_initfn;
+k->realize = igd_pt_i440fx_realize;
dc->desc = "IGD Passthrough Host bridge";
}
--
Yours Sincerely,
Cao Jin
Re: [Qemu-devel] [PATCH] change type of pci_bridge_initfn() to void
Hi mst
friendly ping again...
On 12/17/2015 09:53 AM, Cao jin wrote:
Ping
On 11/30/2015 05:19 PM, Michael S. Tsirkin wrote:
On Mon, Nov 30, 2015 at 05:00:44PM +0800, Cao jin wrote:
It always return 0(success), change its type to void, and modify its
caller.
Doing this can reduce a error path of its caller, and it is also good
when
convert init() to realize()
Signed-off-by: Cao jin
Sounds good, but pls remember to ping me after 2.5 is out.
---
hw/pci-bridge/i82801b11.c | 5 +
hw/pci-bridge/ioh3420.c| 6 +-
hw/pci-bridge/pci_bridge_dev.c | 8 +++-
hw/pci-bridge/xio3130_downstream.c | 6 +-
hw/pci-bridge/xio3130_upstream.c | 6 +-
hw/pci-host/apb.c | 5 +
hw/pci/pci_bridge.c| 3 +--
include/hw/pci/pci_bridge.h| 2 +-
8 files changed, 10 insertions(+), 31 deletions(-)
leave DEC 21154 PCI-PCI bridge unchanged because it is better to
handle it
when convert init() to realize().
diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c
index 7e79bc0..b21bc2c 100644
--- a/hw/pci-bridge/i82801b11.c
+++ b/hw/pci-bridge/i82801b11.c
@@ -61,10 +61,7 @@ static int i82801b11_bridge_initfn(PCIDevice *d)
{
int rc;
-rc = pci_bridge_initfn(d, TYPE_PCI_BUS);
-if (rc < 0) {
-return rc;
-}
+pci_bridge_initfn(d, TYPE_PCI_BUS);
rc = pci_bridge_ssvid_init(d, I82801ba_SSVID_OFFSET,
I82801ba_SSVID_SVID,
I82801ba_SSVID_SSID);
diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
index cce2fdd..eead195 100644
--- a/hw/pci-bridge/ioh3420.c
+++ b/hw/pci-bridge/ioh3420.c
@@ -97,11 +97,7 @@ static int ioh3420_initfn(PCIDevice *d)
PCIESlot *s = PCIE_SLOT(d);
int rc;
-rc = pci_bridge_initfn(d, TYPE_PCIE_BUS);
-if (rc < 0) {
-return rc;
-}
-
+pci_bridge_initfn(d, TYPE_PCIE_BUS);
pcie_port_init_reg(d);
rc = pci_bridge_ssvid_init(d, IOH_EP_SSVID_OFFSET,
diff --git a/hw/pci-bridge/pci_bridge_dev.c
b/hw/pci-bridge/pci_bridge_dev.c
index 26aded9..bc3e1b7 100644
--- a/hw/pci-bridge/pci_bridge_dev.c
+++ b/hw/pci-bridge/pci_bridge_dev.c
@@ -52,10 +52,8 @@ static int pci_bridge_dev_initfn(PCIDevice *dev)
PCIBridgeDev *bridge_dev = PCI_BRIDGE_DEV(dev);
int err;
-err = pci_bridge_initfn(dev, TYPE_PCI_BUS);
-if (err) {
-goto bridge_error;
-}
+pci_bridge_initfn(dev, TYPE_PCI_BUS);
+
if (bridge_dev->flags & (1 << PCI_BRIDGE_DEV_F_SHPC_REQ)) {
dev->config[PCI_INTERRUPT_PIN] = 0x1;
memory_region_init(&bridge_dev->bar, OBJECT(dev), "shpc-bar",
@@ -94,7 +92,7 @@ slotid_error:
}
shpc_error:
pci_bridge_exitfn(dev);
-bridge_error:
+
return err;
}
diff --git a/hw/pci-bridge/xio3130_downstream.c
b/hw/pci-bridge/xio3130_downstream.c
index 86b7970..012daf3 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -61,11 +61,7 @@ static int xio3130_downstream_initfn(PCIDevice *d)
PCIESlot *s = PCIE_SLOT(d);
int rc;
-rc = pci_bridge_initfn(d, TYPE_PCIE_BUS);
-if (rc < 0) {
-return rc;
-}
-
+pci_bridge_initfn(d, TYPE_PCIE_BUS);
pcie_port_init_reg(d);
rc = msi_init(d, XIO3130_MSI_OFFSET, XIO3130_MSI_NR_VECTOR,
diff --git a/hw/pci-bridge/xio3130_upstream.c
b/hw/pci-bridge/xio3130_upstream.c
index eada582..434c8fd 100644
--- a/hw/pci-bridge/xio3130_upstream.c
+++ b/hw/pci-bridge/xio3130_upstream.c
@@ -56,11 +56,7 @@ static int xio3130_upstream_initfn(PCIDevice *d)
PCIEPort *p = PCIE_PORT(d);
int rc;
-rc = pci_bridge_initfn(d, TYPE_PCIE_BUS);
-if (rc < 0) {
-return rc;
-}
-
+pci_bridge_initfn(d, TYPE_PCIE_BUS);
pcie_port_init_reg(d);
rc = msi_init(d, XIO3130_MSI_OFFSET, XIO3130_MSI_NR_VECTOR,
diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index 599768e..e9117b9 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -636,10 +636,7 @@ static int apb_pci_bridge_initfn(PCIDevice *dev)
{
int rc;
-rc = pci_bridge_initfn(dev, TYPE_PCI_BUS);
-if (rc < 0) {
-return rc;
-}
+pci_bridge_initfn(dev, TYPE_PCI_BUS);
/*
* command register:
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index 40c97b1..5c30795 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -332,7 +332,7 @@ void pci_bridge_reset(DeviceState *qdev)
}
/* default qdev initialization function for PCI-to-PCI bridge */
-int pci_bridge_initfn(PCIDevice *dev, const char *typename)
+void pci_bridge_initfn(PCIDevice *dev, const char *typename)
{
PCIBus *parent = dev->bus;
PCIBridge *br = PCI_BRIDGE(dev);
@@ -378,7 +378,6 @@ int pci_bridge_initfn(PCIDevice *dev, const char
*typename)
br->windows = pci_bridge_region_init(br);
QLIST_INIT(&sec_bus->child);
QLIST_INSERT_HEAD(&parent->child, sec_bus, sibling);
-return 0;
}
/* default qdev clean up funct
[Qemu-devel] [PATCH 0/2] Fix some coverity reported defects
The first change replaces QLIST_FOREACH with the safe variant and the second was incorrectly using MTPObject * in the trace function after freeing it. Bandan Das (2): usb-mtp: use safe variant when cleaning events list usb-mtp: fix call to trace function hw/usb/dev-mtp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) -- 2.6.2
Re: [Qemu-devel] [PATCH 3/5] block: added check image option and callback bdrv_is_opened_unclean
On Wed, 12/23 10:46, Denis V. Lunev wrote:
> From: Olga Krishtal
>
> If image is opened for writing and it was not closed correctly
> (the image is dirty) we have to check and repair it. By default
> the option is off.
>
> bdrv_is_opened_unclean - cheks if the image is dirty
> This callbsck will be used to ensure that image was
> closed correctly, and if not - to check and repair it.
>
> Signed-off-by: Olga Krishtal
> Signed-off-by: Denis V. Lunev
> CC: Kevin Wolf
> CC: Max Reitz
> CC: Eric Blake
> CC: Fam Zheng
> ---
> block.c | 32
> include/block/block.h | 1 +
> include/block/block_int.h | 1 +
> 3 files changed, 34 insertions(+)
>
> diff --git a/block.c b/block.c
> index 74228b8..1f704f5 100644
> --- a/block.c
> +++ b/block.c
> @@ -903,6 +903,12 @@ static QemuOptsList bdrv_runtime_opts = {
> .help = "How to lock the image: none or lockfile",
> .def_value_str = "none",
> },
> +{
> +.name = "check",
> +.type = QEMU_OPT_BOOL,
> +.help = "Check and repair the image if it is unclean",
> +.def_value_str = "off",
> +},
> { /* end of list */ }
> },
> };
> @@ -1042,6 +1048,16 @@ static int bdrv_open_common(BlockDriverState *bs,
> BdrvChild *file,
> }
> }
>
> +if (!bs->read_only && qemu_opt_get_bool_del(opts, "check", true) &&
> +bdrv_is_opened_unclean(bs) && !(flags | BDRV_O_CHECK)) {
> +BdrvCheckResult result = {0};
> +ret = bdrv_check(bs, &result, BDRV_FIX_ERRORS);
> +if (ret < 0) {
> +error_setg_errno(errp, -ret, "Could not repair dirty image");
> +bdrv_close(bs);
> +goto fail_opts;
> +}
> +}
> if (bs->encrypted) {
> error_report("Encrypted images are deprecated");
> error_printf("Support for them will be removed in a future
> release.\n"
> @@ -4361,3 +4377,19 @@ int bdrv_lock_image(BlockDriverState *bs,
> BdrvLockImage lock)
> }
> return -EOPNOTSUPP;
> }
> +
> +bool bdrv_is_opened_unclean(BlockDriverState *bs)
> +{
> +BlockDriver *drv = bs->drv;
> +if (drv != NULL && drv->bdrv_is_opened_unclean != NULL) {
> +return drv->bdrv_is_opened_unclean(bs);
> +}
> +if (bs->file == NULL) {
> +return false;
> +}
> +drv = bs->file->bs->drv;
> +if (drv != NULL && drv->bdrv_is_opened_unclean != NULL) {
> +return drv->bdrv_is_opened_unclean;
Should this be
return drv->bdrv_is_opened_unclean(bs);
?
(well, I may be wrong in saying this doesn't compile :)
Fam
> +}
> +return false;
> +}
> diff --git a/include/block/block.h b/include/block/block.h
> index 27fc434..c366990 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -507,6 +507,7 @@ void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, struct
> HBitmapIter *hbi);
> void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
> int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
> int bdrv_lock_image(BlockDriverState *bs, BdrvLockImage lock);
> +bool bdrv_is_opened_unclean(BlockDriverState *bs);
>
> void bdrv_enable_copy_on_read(BlockDriverState *bs);
> void bdrv_disable_copy_on_read(BlockDriverState *bs);
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 755f342..fc3f4a6 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -137,6 +137,7 @@ struct BlockDriver {
> int (*bdrv_make_empty)(BlockDriverState *bs);
> int (*bdrv_lock_image)(BlockDriverState *bs, BdrvLockImage lock);
>
> +bool (*bdrv_is_opened_unclean)(BlockDriverState *bs);
> void (*bdrv_refresh_filename)(BlockDriverState *bs, QDict *options);
>
> /* aio */
> --
> 2.1.4
>
[Qemu-devel] [PATCH 1/2] usb-mtp: use safe variant when cleaning events list
usb_mtp_inotify_cleanup uses QLIST_FOREACH to pick events
from a list and free them which is incorrect. Use QLIST_FOREACH_SAFE
instead.
Signed-off-by: Bandan Das
---
hw/usb/dev-mtp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index af056c7..db1fd59 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -556,7 +556,7 @@ static int usb_mtp_inotify_init(MTPState *s)
static void usb_mtp_inotify_cleanup(MTPState *s)
{
-MTPMonEntry *e;
+MTPMonEntry *e, *p;
if (!s->inotifyfd) {
return;
@@ -565,7 +565,7 @@ static void usb_mtp_inotify_cleanup(MTPState *s)
qemu_set_fd_handler(s->inotifyfd, NULL, NULL, s);
close(s->inotifyfd);
-QTAILQ_FOREACH(e, &s->events, next) {
+QTAILQ_FOREACH_SAFE(e, &s->events, next, p) {
QTAILQ_REMOVE(&s->events, e, next);
g_free(e);
}
--
2.6.2
[Qemu-devel] [PATCH 2/2] usb-mtp: fix call to trace function
trace_usb_mtp_inotify_event() was being called after the object was being freed. Signed-off-by: Bandan Das --- hw/usb/dev-mtp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c index db1fd59..4177a87 100644 --- a/hw/usb/dev-mtp.c +++ b/hw/usb/dev-mtp.c @@ -502,9 +502,9 @@ static void inotify_watchfn(void *arg) entry = g_new0(MTPMonEntry, 1); entry->handle = o->handle; entry->event = EVT_OBJ_REMOVED; -usb_mtp_object_free(s, o); trace_usb_mtp_inotify_event(s->dev.addr, o->path, event->mask, "Obj Deleted"); +usb_mtp_object_free(s, o); break; case IN_MODIFY: -- 2.6.2
Re: [Qemu-devel] [PATCH 3/5] block: added check image option and callback bdrv_is_opened_unclean
On 12/23/2015 12:09 PM, Fam Zheng wrote:
On Wed, 12/23 10:46, Denis V. Lunev wrote:
From: Olga Krishtal
If image is opened for writing and it was not closed correctly
(the image is dirty) we have to check and repair it. By default
the option is off.
bdrv_is_opened_unclean - cheks if the image is dirty
This callbsck will be used to ensure that image was
closed correctly, and if not - to check and repair it.
Signed-off-by: Olga Krishtal
Signed-off-by: Denis V. Lunev
CC: Kevin Wolf
CC: Max Reitz
CC: Eric Blake
CC: Fam Zheng
---
block.c | 32
include/block/block.h | 1 +
include/block/block_int.h | 1 +
3 files changed, 34 insertions(+)
diff --git a/block.c b/block.c
index 74228b8..1f704f5 100644
--- a/block.c
+++ b/block.c
@@ -903,6 +903,12 @@ static QemuOptsList bdrv_runtime_opts = {
.help = "How to lock the image: none or lockfile",
.def_value_str = "none",
},
+{
+.name = "check",
+.type = QEMU_OPT_BOOL,
+.help = "Check and repair the image if it is unclean",
+.def_value_str = "off",
+},
{ /* end of list */ }
},
};
@@ -1042,6 +1048,16 @@ static int bdrv_open_common(BlockDriverState *bs,
BdrvChild *file,
}
}
+if (!bs->read_only && qemu_opt_get_bool_del(opts, "check", true) &&
+bdrv_is_opened_unclean(bs) && !(flags | BDRV_O_CHECK)) {
+BdrvCheckResult result = {0};
+ret = bdrv_check(bs, &result, BDRV_FIX_ERRORS);
+if (ret < 0) {
+error_setg_errno(errp, -ret, "Could not repair dirty image");
+bdrv_close(bs);
+goto fail_opts;
+}
+}
if (bs->encrypted) {
error_report("Encrypted images are deprecated");
error_printf("Support for them will be removed in a future release.\n"
@@ -4361,3 +4377,19 @@ int bdrv_lock_image(BlockDriverState *bs, BdrvLockImage
lock)
}
return -EOPNOTSUPP;
}
+
+bool bdrv_is_opened_unclean(BlockDriverState *bs)
+{
+BlockDriver *drv = bs->drv;
+if (drv != NULL && drv->bdrv_is_opened_unclean != NULL) {
+return drv->bdrv_is_opened_unclean(bs);
+}
+if (bs->file == NULL) {
+return false;
+}
+drv = bs->file->bs->drv;
+if (drv != NULL && drv->bdrv_is_opened_unclean != NULL) {
+return drv->bdrv_is_opened_unclean;
Should this be
return drv->bdrv_is_opened_unclean(bs);
?
(well, I may be wrong in saying this doesn't compile :)
Fam
exactly :)
+}
+return false;
+}
diff --git a/include/block/block.h b/include/block/block.h
index 27fc434..c366990 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -507,6 +507,7 @@ void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, struct
HBitmapIter *hbi);
void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
int bdrv_lock_image(BlockDriverState *bs, BdrvLockImage lock);
+bool bdrv_is_opened_unclean(BlockDriverState *bs);
void bdrv_enable_copy_on_read(BlockDriverState *bs);
void bdrv_disable_copy_on_read(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 755f342..fc3f4a6 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -137,6 +137,7 @@ struct BlockDriver {
int (*bdrv_make_empty)(BlockDriverState *bs);
int (*bdrv_lock_image)(BlockDriverState *bs, BdrvLockImage lock);
+bool (*bdrv_is_opened_unclean)(BlockDriverState *bs);
void (*bdrv_refresh_filename)(BlockDriverState *bs, QDict *options);
/* aio */
--
2.1.4
Re: [Qemu-devel] [PATCH v2 13/14] qapi: Support pretty printing in JSON output visitor
On Mon, 12/21 17:31, Eric Blake wrote: > Similar to pretty printing in the QObject visitor. The rickiest "trickiest"? > parts are the fact that during type_any(), we have to coordinate > with QObject to also print pretty; and the fact that the testsuite > now has to honor parameterization on whether pretty printing is > enabled. > > Signed-off-by: Eric Blake
Re: [Qemu-devel] Minutes from the "Stuttgart block Gipfele"
On Fri, 12/18 14:15, Markus Armbruster wrote: > First, let's examine how such a chain could look like. If we read the > current code correctly, it behaves as if we had a chain > > BB > | > throttle > | > detect-zero > | > copy-on-read > | > BDS > > Except for the backup job, which behaves as if we had > >backup job > / > notifier > | > detect-zero > | > BDS Just to brainstorm block jobs in the dynamic reconfigured node graph: (not sure if this is useful) Nothing stops us from viewing backup as a self-contained filter, [backup] | detect-zero | BDS where its .bdrv_co_writev copies out the old data, and at instantiation time it also creates a long running coroutine (backup_run). In that theory, all other block job types, mirror/stream/commit, fit into a "pull" model, which follows a specified dirty bitmap and copies data from a specified src BDS. In this pull model, mirror (device=d0 target=d1) becomes a pull fileter: BB[d0]BB[d1] | | throttle[pull,src=d0] | | detect-zero detect-zero | | copy-on-read copy-on-read | | BDS BDS Note: the pull reuses most of the block/mirror.c code except the s->dirty_bitmap will be initialized depending on the block job type. In the case of mirror, it is trivially the same as now. stream (device=d0 base=base0) becomes a pull filter: BB[d0] | [pull,src=base0] | detect-zero | copy-on-read | BDS | BDS[base0] Note: s->dirty_bitmap will be initialized with the blocks which should be copied by block-stream. Similarly, active commit (device=d0 base=base0) becomes a pull filter: BB[d0] | detect-zero | copy-on-read | BDS | [pull,src=d0] | BDS[base0] and commit (device=d0 top=base1 base=base0) becomes a pull filter: BB[d0] | detect-zero | copy-on-read | BDS | BDS[base1] | [pull,src=base1] | BDS[base0] If this could work, I'm looking forward to a pretty looking diffstat if we can unify the coroutine code of all four jobs. :) Fam
[Qemu-devel] [PATCH] Xen PCI passthrough: convert to realize()
Signed-off-by: Cao jin
---
Since the callchain is pretty deep & error path is very much too, so I made the
patch based on the principal: catch/report the most necessary error msg with
smallest modification.(So you can see I don`t change some functions to void,
despite they coule be)
hw/xen/xen-host-pci-device.c | 79 +++-
hw/xen/xen-host-pci-device.h | 5 +--
hw/xen/xen_pt.c | 67 +++--
hw/xen/xen_pt.h | 5 +--
hw/xen/xen_pt_config_init.c | 47 +-
hw/xen/xen_pt_graphics.c | 6 ++--
6 files changed, 118 insertions(+), 91 deletions(-)
diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
index 7d8a023..1ab6d97 100644
--- a/hw/xen/xen-host-pci-device.c
+++ b/hw/xen/xen-host-pci-device.c
@@ -43,13 +43,14 @@ static int xen_host_pci_sysfs_path(const XenHostPCIDevice
*d,
/* The output is truncated, or some other error was encountered */
return -ENODEV;
}
+
return 0;
}
/* This size should be enough to read the first 7 lines of a resource file */
#define XEN_HOST_PCI_RESOURCE_BUFFER_SIZE 400
-static int xen_host_pci_get_resource(XenHostPCIDevice *d)
+static void xen_host_pci_get_resource(XenHostPCIDevice *d, Error **errp)
{
int i, rc, fd;
char path[PATH_MAX];
@@ -60,23 +61,24 @@ static int xen_host_pci_get_resource(XenHostPCIDevice *d)
rc = xen_host_pci_sysfs_path(d, "resource", path, sizeof (path));
if (rc) {
-return rc;
+error_setg_errno(errp, errno, "snprintf err");
+return;
}
+
fd = open(path, O_RDONLY);
if (fd == -1) {
-XEN_HOST_PCI_LOG("Error: Can't open %s: %s\n", path, strerror(errno));
-return -errno;
+error_setg_errno(errp, errno, "open err");
+return;
}
do {
rc = read(fd, &buf, sizeof (buf) - 1);
if (rc < 0 && errno != EINTR) {
-rc = -errno;
+error_setg_errno(errp, errno, "read err");
goto out;
}
} while (rc < 0);
buf[rc] = 0;
-rc = 0;
s = buf;
for (i = 0; i < PCI_NUM_REGIONS; i++) {
@@ -129,20 +131,20 @@ static int xen_host_pci_get_resource(XenHostPCIDevice *d)
d->rom.bus_flags = flags & IORESOURCE_BITS;
}
}
+
if (i != PCI_NUM_REGIONS) {
/* Invalid format or input to short */
-rc = -ENODEV;
+error_setg(errp, "Invalid format or input to short");
}
out:
close(fd);
-return rc;
}
/* This size should be enough to read a long from a file */
#define XEN_HOST_PCI_GET_VALUE_BUFFER_SIZE 22
static int xen_host_pci_get_value(XenHostPCIDevice *d, const char *name,
- unsigned int *pvalue, int base)
+ unsigned int *pvalue, int base, Error **errp)
{
char path[PATH_MAX];
char buf[XEN_HOST_PCI_GET_VALUE_BUFFER_SIZE];
@@ -152,30 +154,38 @@ static int xen_host_pci_get_value(XenHostPCIDevice *d,
const char *name,
rc = xen_host_pci_sysfs_path(d, name, path, sizeof (path));
if (rc) {
+error_setg_errno(errp, errno, "snprintf err");
return rc;
}
+
fd = open(path, O_RDONLY);
if (fd == -1) {
-XEN_HOST_PCI_LOG("Error: Can't open %s: %s\n", path, strerror(errno));
+error_setg_errno(errp, errno, "open err");
return -errno;
}
+
do {
rc = read(fd, &buf, sizeof (buf) - 1);
if (rc < 0 && errno != EINTR) {
+error_setg_errno(errp, errno, "read err");
rc = -errno;
goto out;
}
} while (rc < 0);
+
buf[rc] = 0;
value = strtol(buf, &endptr, base);
if (endptr == buf || *endptr != '\n') {
+error_setg(errp, "format invalid");
rc = -1;
} else if ((value == LONG_MIN || value == LONG_MAX) && errno == ERANGE) {
+error_setg_errno(errp, errno, "strtol err");
rc = -errno;
} else {
rc = 0;
*pvalue = value;
}
+
out:
close(fd);
return rc;
@@ -183,16 +193,18 @@ out:
static inline int xen_host_pci_get_hex_value(XenHostPCIDevice *d,
const char *name,
- unsigned int *pvalue)
+ unsigned int *pvalue,
+ Error **errp)
{
-return xen_host_pci_get_value(d, name, pvalue, 16);
+return xen_host_pci_get_value(d, name, pvalue, 16, errp);
}
static inline int xen_host_pci_get_dec_value(XenHostPCIDevice *d,
const char *name,
- unsigned int *pvalue)
+ unsigned int *pvalue,
+ Error **errp)
{
-return xen_host_pci_get_value(d, name, pvalu
[Qemu-devel] [PATCH] qemu-iotests: make check-block.sh work on out-of-tree builds
Since check-block.sh, the "check" script has learnt to find the source path. On the other hand, it expects common.env to be in the build tree (both changes made in commit 76c7560, "configure: Enable out-of-tree iotests", 2014-05-24). So, it is wrong to invoke "check" from the source path like check-block.sh does. Fix it. Signed-off-by: Paolo Bonzini --- tests/check-block.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/check-block.sh b/tests/check-block.sh index b9d9c6a..a37797a 100755 --- a/tests/check-block.sh +++ b/tests/check-block.sh @@ -9,7 +9,7 @@ if [ ! -x $QEMU_PROG ]; then exit 1 fi -cd $SRC_PATH/tests/qemu-iotests +cd tests/qemu-iotests ret=0 ./check -T -nocache -raw || ret=1 -- 2.5.0
Re: [Qemu-devel] [PATCH 00/10] qcow2: Implement image locking
On Wed, Dec 23, 2015 at 11:14:12AM +0800, Fam Zheng wrote: > On Tue, 12/22 17:46, Kevin Wolf wrote: > > Enough innocent images have died because users called 'qemu-img snapshot' > > while > > the VM was still running. Educating the users doesn't seem to be a working > > strategy, so this series adds locking to qcow2 that refuses to access the > > image > > read-write from two processes. > > > > Eric, this will require a libvirt update to deal with qemu crashes which > > leave > > locked images behind. The simplest thinkable way would be to unconditionally > > override the lock in libvirt whenever the option is present. In that case, > > libvirt VMs would be protected against concurrent non-libvirt accesses, but > > not > > the other way round. If you want more than that, libvirt would have to check > > somehow if it was its own VM that used the image and left the lock behind. I > > imagine that can't be too hard either. > > The motivation is great, but I'm not sure I like the side-effect that an > unclean shutdown will require a "forced" open, because it makes using qcow2 in > development cumbersome, and like you said, management/user also needs to > handle > this explicitly. This is a bit of a personal preference, but it's strong > enough > that I want to speak up. Yeah, I am also not really a big fan of locking mechanisms which are not automatically cleaned up on process exit. On the other hand you could say that people who choose to run qemu-img manually are already taking fate into their own hands, and ending up with a dirty image on unclean exit is still miles better than loosing all your data. > As an alternative, can we introduce .bdrv_flock() in protocol drivers, with > similar semantics to flock(2) or lockf(3)? That way all formats can benefit, > and a program crash will automatically drop the lock. FWIW, the libvirt locking daemon (virtlockd) will already attempt to take out locks using fcntl()/lockf() on all disk images associated with a VM. This only protects against two QEMU emulators running at the same time though, and also only if they're using libvirt APIs. So it doesn't protect if someone runs qemu-img manually, or indeed if libvirt runs qemu-img, though we could fairly easily address the latter. A problem with lockf is that it is almost unusable by design, because if you have 2 (or more) file descriptors open against the same file, if you close *any* of the file descriptors it releases all locks on the file, even if the locks were acquired on a different file descriptor than the one being closed :-( This is why we put our locking code into a completely separate process (virtlockd), to guarantee nothing else might accidentally open/close file descriptors on the same file we had locked. A second problem with using flock/lockf() is that on block devices the locks are only scoped to the local host, so if you have shared block storage they locks are not all that useful. To deal with this, virtlockd has the concept of a "lockspace". The default lockspace is associated directly while the disk files, but alternate lockspaces are possible which are indirectly associated. For example, we have lockspaces that are keyed off the SCSI unique volume ID, and the LVM volume UUID, which cna be placed on a shared filesystem. This lets us get cross-host locking even for block storage. We have a future desire to be able to make use of storage native locking mechansisms too such as SCSI reservations. So while QEMU could add a bdrv_lock() driver method, it will have some limitations & implementation complexity (ensuring nothing else in QEMU can ever accidentally open+close the same file that QEMU has locked), though it could offer better protection than we have with libvirt for cases whe e people run qemu-img manually. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
[Qemu-devel] [PATCH] block: use drained section in bdrv_close
bdrv_close is used when ejecting a medium. Use a drained section to ensure
that all I/O goes to either the old medium or the bitbucket.
Signed-off-by: Paolo Bonzini
---
block.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index 411edbf..01655de 100644
--- a/block.c
+++ b/block.c
@@ -2154,9 +2154,10 @@ void bdrv_close(BlockDriverState *bs)
bdrv_io_limits_disable(bs);
}
-bdrv_drain(bs); /* complete I/O */
+bdrv_drained_begin(bs); /* complete I/O */
bdrv_flush(bs);
bdrv_drain(bs); /* in case flush left pending I/O */
+
notifier_list_notify(&bs->close_notifiers, bs);
if (bs->blk) {
@@ -2206,6 +2207,7 @@ void bdrv_close(BlockDriverState *bs)
g_free(ban);
}
QLIST_INIT(&bs->aio_notifiers);
+bdrv_drained_end(bs);
}
void bdrv_close_all(void)
--
2.5.0
[Qemu-devel] [PATCH] block: add missing call to bdrv_drain_recurse
This is also needed in bdrv_drain_all, not just in bdrv_drain.
Signed-off-by: Paolo Bonzini
---
block/io.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/io.c b/block/io.c
index 841f5b5..bfe2544 100644
--- a/block/io.c
+++ b/block/io.c
@@ -293,6 +293,7 @@ void bdrv_drain_all(void)
if (bs->job) {
block_job_pause(bs->job);
}
+bdrv_drain_recurse(bs);
aio_context_release(aio_context);
if (!g_slist_find(aio_ctxs, aio_context)) {
--
2.5.0
[Qemu-devel] [PATCH] blockjob: prevent double enter (with dangling access on the second)
Because block_job_sleep_ns marks the job as non-busy, it is
possible to enter it again from block_job_enter. However, the
same coroutine may then be re-entered from co_sleep_cb, and this
time the CoSleepCB is not on the stack anymore.
Fix this by open coding the sleep in block_job_sleep_ns, and
deleting the timer immediately after the coroutine is re-entered.
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini
---
blockjob.c | 27 +++
include/block/blockjob.h | 3 +--
2 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/blockjob.c b/blockjob.c
index 80adb9d..5e59a8f 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -329,8 +329,21 @@ int block_job_complete_sync(BlockJob *job, Error **errp)
return block_job_finish_sync(job, &block_job_complete, errp);
}
+typedef struct CoSleepCB {
+QEMUTimer *ts;
+Coroutine *co;
+} CoSleepCB;
+
+static void co_sleep_cb(void *opaque)
+{
+CoSleepCB *sleep_cb = opaque;
+
+qemu_coroutine_enter(sleep_cb->co, NULL);
+}
+
void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns)
{
+CoSleepCB sleep_cb = { .ts = NULL };
assert(job->busy);
/* Check cancellation *before* setting busy = false, too! */
@@ -339,10 +352,16 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType
type, int64_t ns)
}
job->busy = false;
-if (block_job_is_paused(job)) {
-qemu_coroutine_yield();
-} else {
-co_aio_sleep_ns(bdrv_get_aio_context(job->bs), type, ns);
+if (!block_job_is_paused(job)) {
+sleep_cb.co = qemu_coroutine_self(),
+sleep_cb.ts = aio_timer_new(bdrv_get_aio_context(job->bs), type,
+SCALE_NS, co_sleep_cb, &sleep_cb);
+timer_mod(sleep_cb.ts, qemu_clock_get_ns(type) + ns);
+}
+qemu_coroutine_yield();
+if (sleep_cb.ts) {
+timer_del(sleep_cb.ts);
+timer_free(sleep_cb.ts);
}
job->busy = true;
}
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index d84ccd8..82063f2 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -120,8 +120,7 @@ struct BlockJob {
/**
* Set to false by the job while it is in a quiescent state, where
- * no I/O is pending and the job has yielded on any condition
- * that is not detected by #aio_poll, such as a timer.
+ * no I/O is pending. The job can be reentered with qemu_coroutine_enter.
*/
bool busy;
--
2.5.0
[Qemu-devel] [PATCH] block: acquire in bdrv_query_image_info
NFS calls aio_poll inside bdrv_get_allocated_size. This requires
acquiring the AioContext.
Signed-off-by: Paolo Bonzini
---
block/qapi.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index fecac25..ea400e0 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -210,11 +210,13 @@ void bdrv_query_image_info(BlockDriverState *bs,
Error *err = NULL;
ImageInfo *info;
+aio_context_acquire(bdrv_get_aio_context(bs));
+
size = bdrv_getlength(bs);
if (size < 0) {
error_setg_errno(errp, -size, "Can't get size of device '%s'",
bdrv_get_device_name(bs));
-return;
+goto out;
}
info = g_new0(ImageInfo, 1);
@@ -281,10 +283,13 @@ void bdrv_query_image_info(BlockDriverState *bs,
default:
error_propagate(errp, err);
qapi_free_ImageInfo(info);
-return;
+goto out;
}
*p_info = info;
+
+out:
+aio_context_release(bdrv_get_aio_context(bs));
}
/* @p_info will be set only on success. */
--
2.5.0
Re: [Qemu-devel] [PATCH 1/2] io: fix setting of QIO_CHANNEL_FEATURE_FD_PASS on server connections
On Tue, Dec 22, 2015 at 11:14:41AM -0700, Eric Blake wrote:
> On 12/21/2015 09:23 AM, Daniel P. Berrange wrote:
> > The QIO_CHANNEL_FEATURE_FD_PASS feature flag is set in the
> > qio_channel_socket_set_fd() method, however, this only deals
> > with client side connections.
> >
> > To ensure server side connections also have the feature flag
> > set, we must set it in qio_channel_socket_accept() too.
> >
> > Signed-off-by: Daniel P. Berrange
> > ---
> > io/channel-socket.c| 10 --
> > tests/test-io-channel-socket.c | 29 +
> > 2 files changed, 33 insertions(+), 6 deletions(-)
> >
> > diff --git a/io/channel-socket.c b/io/channel-socket.c
> > index 90b3c73..eed2ff5 100644
> > --- a/io/channel-socket.c
> > +++ b/io/channel-socket.c
> > @@ -352,13 +352,19 @@ qio_channel_socket_accept(QIOChannelSocket *ioc,
> > goto error;
> > }
> >
> > -if (getsockname(cioc->fd, (struct sockaddr *)&ioc->localAddr,
> > -&ioc->localAddrLen) < 0) {
> > +if (getsockname(cioc->fd, (struct sockaddr *)&cioc->localAddr,
> > +&cioc->localAddrLen) < 0) {
>
> Looks like a typo fix while at it.
Yes, the latter fix exposed the need for the former fix. I'll mention this
in the commit message too
> Reviewed-by: Eric Blake
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH 2/2] io: fix stack allocation when sending of file descriptors
On Tue, Dec 22, 2015 at 11:20:30AM -0700, Eric Blake wrote:
> On 12/21/2015 09:23 AM, Daniel P. Berrange wrote:
> > When sending file descriptors over a socket, we have to
> > allocate a data buffer to hold the FDs in the scmsghdr.
> > Unfortunately we allocated the buffer on the stack inside
> > an if () {} block, but called sendmsg() outside the block.
> > So the stack bytes holding the FDs were liable to be
> > overwritten with other data. By luck this was not a problem
> > when sending 1 FD, but if sending 2 or more then it would
> > fail.
> >
> > The fix is to simply move the variables outside the nested
> > 'if' block. To keep valgrind quiet we also zero-initialize
> > the 'control' buffer.
> >
> > Signed-off-by: Daniel P. Berrange
> > ---
> > io/channel-socket.c| 7 ++-
> > tests/test-io-channel-socket.c | 98
> > ++
> > 2 files changed, 101 insertions(+), 4 deletions(-)
> >
>
> The fix itself is obvious from the commit message; the bulk of this
> patch is the testsuite addition (which is a GOOD thing - thanks!).
Yes, I wasted lots of time trying to find the flaw before
I wrote the test case at which point it was trivial to
find with valgrind :-)
>
> > +qio_channel_readv_full(dst,
> > + iorecv,
> > + G_N_ELEMENTS(iorecv),
> > + &fdrecv,
> > + &nfdrecv,
> > + &error_abort);
> > +
> > +g_assert(nfdrecv == G_N_ELEMENTS(fdsend));
> > +/* Each recvd FD should be different from sent FD */
> > +for (i = 0; i < nfdrecv; i++) {
> > +g_assert_cmpint(fdrecv[i], !=, testfd);
> > +}
>
> Here, you blindly dereference fdrecv[]...
>
> > +unlink(TEST_FILE);
> > +close(testfd);
> > +if (fdrecv != NULL) {
>
> ...so this if() is dead, and you can just always do the cleanup.
Yep, will fix
> That's minor, so:
> Reviewed-by: Eric Blake
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH] blockjob: prevent double enter (with dangling access on the second)
On 23/12/2015 11:48, Paolo Bonzini wrote:
> Because block_job_sleep_ns marks the job as non-busy, it is
> possible to enter it again from block_job_enter. However, the
> same coroutine may then be re-entered from co_sleep_cb, and this
> time the CoSleepCB is not on the stack anymore.
>
> Fix this by open coding the sleep in block_job_sleep_ns, and
> deleting the timer immediately after the coroutine is re-entered.
>
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Paolo Bonzini
Forget about this, the current code does exactly the same as what I
wrote, and is correct. /me needs vacation. :)
Paolo
> ---
> blockjob.c | 27 +++
> include/block/blockjob.h | 3 +--
> 2 files changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/blockjob.c b/blockjob.c
> index 80adb9d..5e59a8f 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -329,8 +329,21 @@ int block_job_complete_sync(BlockJob *job, Error **errp)
> return block_job_finish_sync(job, &block_job_complete, errp);
> }
>
> +typedef struct CoSleepCB {
> +QEMUTimer *ts;
> +Coroutine *co;
> +} CoSleepCB;
> +
> +static void co_sleep_cb(void *opaque)
> +{
> +CoSleepCB *sleep_cb = opaque;
> +
> +qemu_coroutine_enter(sleep_cb->co, NULL);
> +}
> +
> void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns)
> {
> +CoSleepCB sleep_cb = { .ts = NULL };
> assert(job->busy);
>
> /* Check cancellation *before* setting busy = false, too! */
> @@ -339,10 +352,16 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType
> type, int64_t ns)
> }
>
> job->busy = false;
> -if (block_job_is_paused(job)) {
> -qemu_coroutine_yield();
> -} else {
> -co_aio_sleep_ns(bdrv_get_aio_context(job->bs), type, ns);
> +if (!block_job_is_paused(job)) {
> +sleep_cb.co = qemu_coroutine_self(),
> +sleep_cb.ts = aio_timer_new(bdrv_get_aio_context(job->bs), type,
> +SCALE_NS, co_sleep_cb, &sleep_cb);
> +timer_mod(sleep_cb.ts, qemu_clock_get_ns(type) + ns);
> +}
> +qemu_coroutine_yield();
> +if (sleep_cb.ts) {
> +timer_del(sleep_cb.ts);
> +timer_free(sleep_cb.ts);
> }
> job->busy = true;
> }
> diff --git a/include/block/blockjob.h b/include/block/blockjob.h
> index d84ccd8..82063f2 100644
> --- a/include/block/blockjob.h
> +++ b/include/block/blockjob.h
> @@ -120,8 +120,7 @@ struct BlockJob {
>
> /**
> * Set to false by the job while it is in a quiescent state, where
> - * no I/O is pending and the job has yielded on any condition
> - * that is not detected by #aio_poll, such as a timer.
> + * no I/O is pending. The job can be reentered with
> qemu_coroutine_enter.
> */
> bool busy;
>
>
Re: [Qemu-devel] [PATCH 1/6] crypto: add ability to query the cipher key, block & IV lens
On Mon, Dec 21, 2015 at 09:18:42AM -0700, Eric Blake wrote: > On 12/21/2015 09:06 AM, Daniel P. Berrange wrote: > > Adds new methods to allow querying the length of the cipher > > key, block size and initialization vectors. > > In the subject line, I read 'lens' as a synonym for 'viewports', not > 'lengths'. But I don't know if avoiding the abbreviation is worth it, > because the subject line is already bordering on long. Maybe avoiding > it altogether is easier? > > crypto: Add additional query accessors Yes, I'll simplify this to "crypto: add additional query accessors for cipher instances" > > > > > Signed-off-by: Daniel P. Berrange > > --- > > crypto/cipher.c| 48 > > ++ > > include/crypto/cipher.h| 37 +++ > > tests/test-crypto-cipher.c | 10 ++ > > 3 files changed, 95 insertions(+) > > > > But no problems with the actual patch, so: > Reviewed-by: Eric Blake Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH 2/6] crypto: add ability to query hash digest len
On Mon, Dec 21, 2015 at 09:22:42AM -0700, Eric Blake wrote:
> On 12/21/2015 09:06 AM, Daniel P. Berrange wrote:
> > Add a qcrypto_hash_digest_len() method which allows querying of
> > the raw digest size for a given hash algorithm.
> >
> > Signed-off-by: Daniel P. Berrange
> > ---
> > crypto/hash.c| 15 +++
> > include/crypto/hash.h| 11 +++
> > tests/test-crypto-hash.c | 5 +
> > 3 files changed, 31 insertions(+)
> >
>
> > +++ b/tests/test-crypto-hash.c
> > @@ -163,6 +163,11 @@ static void test_hash_digest(void)
> > for (i = 0; i < G_N_ELEMENTS(expected_outputs) ; i++) {
> > int ret;
> > char *digest;
> > +size_t digestsize;
> > +
> > +digestsize = qcrypto_hash_digest_len(i);
> > +
> > +g_assert((digestsize * 2) == strlen(expected_outputs[i]));
>
> g_assert_cmpint() might be better here. But that's minor.
Ok, I've changed that.
> Reviewed-by: Eric Blake
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
[Qemu-devel] [PULL v1 1/3] io: bind to loopback IP addrs in test suite
The test suite currently binds to 0.0.0.0 or ::, which covers
all interfaces of the machine. It is bad practice for test
suite to open publically accessible ports on a machine, so
switch to use loopback addrs 127.0.0.1 or ::1.
Reported-by: Peter Maydell
Reviewed-by: Eric Blake
Signed-off-by: Daniel P. Berrange
---
tests/test-io-channel-socket.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/test-io-channel-socket.c b/tests/test-io-channel-socket.c
index 194d043..e76d54c 100644
--- a/tests/test-io-channel-socket.c
+++ b/tests/test-io-channel-socket.c
@@ -261,7 +261,7 @@ static void test_io_channel_ipv4(bool async)
listen_addr->type = SOCKET_ADDRESS_KIND_INET;
listen_addr->u.inet = g_new0(InetSocketAddress, 1);
-listen_addr->u.inet->host = g_strdup("0.0.0.0");
+listen_addr->u.inet->host = g_strdup("127.0.0.1");
listen_addr->u.inet->port = NULL; /* Auto-select */
connect_addr->type = SOCKET_ADDRESS_KIND_INET;
@@ -295,7 +295,7 @@ static void test_io_channel_ipv6(bool async)
listen_addr->type = SOCKET_ADDRESS_KIND_INET;
listen_addr->u.inet = g_new0(InetSocketAddress, 1);
-listen_addr->u.inet->host = g_strdup("::");
+listen_addr->u.inet->host = g_strdup("::1");
listen_addr->u.inet->port = NULL; /* Auto-select */
connect_addr->type = SOCKET_ADDRESS_KIND_INET;
--
2.5.0
[Qemu-devel] [PULL v1 2/3] io: fix setting of QIO_CHANNEL_FEATURE_FD_PASS on server connections
The QIO_CHANNEL_FEATURE_FD_PASS feature flag is set in the
qio_channel_socket_set_fd() method, however, this only deals
with client side connections.
To ensure server side connections also have the feature flag
set, we must set it in qio_channel_socket_accept() too. This
also highlighted a typo fix where the code updated the
sockaddr struct in the wrong object instance.
Reviewed-by: Eric Blake
Signed-off-by: Daniel P. Berrange
---
io/channel-socket.c| 10 --
tests/test-io-channel-socket.c | 29 +
2 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/io/channel-socket.c b/io/channel-socket.c
index 90b3c73..eed2ff5 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -352,13 +352,19 @@ qio_channel_socket_accept(QIOChannelSocket *ioc,
goto error;
}
-if (getsockname(cioc->fd, (struct sockaddr *)&ioc->localAddr,
-&ioc->localAddrLen) < 0) {
+if (getsockname(cioc->fd, (struct sockaddr *)&cioc->localAddr,
+&cioc->localAddrLen) < 0) {
error_setg_errno(errp, socket_error(),
"Unable to query local socket address");
goto error;
}
+#ifndef WIN32
+if (cioc->localAddr.ss_family == AF_UNIX) {
+QIO_CHANNEL(cioc)->features |= (1 << QIO_CHANNEL_FEATURE_FD_PASS);
+}
+#endif /* WIN32 */
+
trace_qio_channel_socket_accept_complete(ioc, cioc, cioc->fd);
return cioc;
diff --git a/tests/test-io-channel-socket.c b/tests/test-io-channel-socket.c
index e76d54c..b0eb538 100644
--- a/tests/test-io-channel-socket.c
+++ b/tests/test-io-channel-socket.c
@@ -210,13 +210,19 @@ static void test_io_channel_setup_async(SocketAddress
*listen_addr,
static void test_io_channel(bool async,
SocketAddress *listen_addr,
-SocketAddress *connect_addr)
+SocketAddress *connect_addr,
+bool passFD)
{
QIOChannel *src, *dst;
QIOChannelTest *test;
if (async) {
test_io_channel_setup_async(listen_addr, connect_addr, &src, &dst);
+g_assert(!passFD ||
+ qio_channel_has_feature(src, QIO_CHANNEL_FEATURE_FD_PASS));
+g_assert(!passFD ||
+ qio_channel_has_feature(dst, QIO_CHANNEL_FEATURE_FD_PASS));
+
test = qio_channel_test_new();
qio_channel_test_run_threads(test, true, src, dst);
qio_channel_test_validate(test);
@@ -226,6 +232,11 @@ static void test_io_channel(bool async,
test_io_channel_setup_async(listen_addr, connect_addr, &src, &dst);
+g_assert(!passFD ||
+ qio_channel_has_feature(src, QIO_CHANNEL_FEATURE_FD_PASS));
+g_assert(!passFD ||
+ qio_channel_has_feature(dst, QIO_CHANNEL_FEATURE_FD_PASS));
+
test = qio_channel_test_new();
qio_channel_test_run_threads(test, false, src, dst);
qio_channel_test_validate(test);
@@ -235,6 +246,11 @@ static void test_io_channel(bool async,
} else {
test_io_channel_setup_sync(listen_addr, connect_addr, &src, &dst);
+g_assert(!passFD ||
+ qio_channel_has_feature(src, QIO_CHANNEL_FEATURE_FD_PASS));
+g_assert(!passFD ||
+ qio_channel_has_feature(dst, QIO_CHANNEL_FEATURE_FD_PASS));
+
test = qio_channel_test_new();
qio_channel_test_run_threads(test, true, src, dst);
qio_channel_test_validate(test);
@@ -244,6 +260,11 @@ static void test_io_channel(bool async,
test_io_channel_setup_sync(listen_addr, connect_addr, &src, &dst);
+g_assert(!passFD ||
+ qio_channel_has_feature(src, QIO_CHANNEL_FEATURE_FD_PASS));
+g_assert(!passFD ||
+ qio_channel_has_feature(dst, QIO_CHANNEL_FEATURE_FD_PASS));
+
test = qio_channel_test_new();
qio_channel_test_run_threads(test, false, src, dst);
qio_channel_test_validate(test);
@@ -269,7 +290,7 @@ static void test_io_channel_ipv4(bool async)
connect_addr->u.inet->host = g_strdup("127.0.0.1");
connect_addr->u.inet->port = NULL; /* Filled in later */
-test_io_channel(async, listen_addr, connect_addr);
+test_io_channel(async, listen_addr, connect_addr, false);
qapi_free_SocketAddress(listen_addr);
qapi_free_SocketAddress(connect_addr);
@@ -303,7 +324,7 @@ static void test_io_channel_ipv6(bool async)
connect_addr->u.inet->host = g_strdup("::1");
connect_addr->u.inet->port = NULL; /* Filled in later */
-test_io_channel(async, listen_addr, connect_addr);
+test_io_channel(async, listen_addr, connect_addr, false);
qapi_free_SocketAddress(listen_addr);
qapi_free_SocketAddress(connect_addr);
@@ -337,7 +358,7 @@ static void test_io_channel_unix(bool async)
connect_addr->u.q_unix = g_new0(UnixSocketAddress, 1);
connect_addr->u.q_unix->path = g_strdup(TEST_SOCKE
[Qemu-devel] [PULL v1 0/3] Misc I/O channel fixes
The following changes since commit 5dc42c186d63b7b338594fc071cf290805dcc5a5: Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2015-12-22 14:21:42 +) are available in the git repository at: git://github.com/berrange/qemu tags/pull-io-fixes-2015-12-23-1 for you to fetch changes up to 7b3c618ad0cd0154993b5b5dbd34e0010960585a: io: fix stack allocation when sending of file descriptors (2015-12-23 10:53:03 +) Merge misc I/O channel fixes Daniel P. Berrange (3): io: bind to loopback IP addrs in test suite io: fix setting of QIO_CHANNEL_FEATURE_FD_PASS on server connections io: fix stack allocation when sending of file descriptors io/channel-socket.c| 17 -- tests/test-io-channel-socket.c | 129 +++-- 2 files changed, 134 insertions(+), 12 deletions(-) -- 2.5.0
[Qemu-devel] [PULL v1 3/3] io: fix stack allocation when sending of file descriptors
When sending file descriptors over a socket, we have to
allocate a data buffer to hold the FDs in the scmsghdr.
Unfortunately we allocated the buffer on the stack inside
an if () {} block, but called sendmsg() outside the block.
So the stack bytes holding the FDs were liable to be
overwritten with other data. By luck this was not a problem
when sending 1 FD, but if sending 2 or more then it would
fail.
The fix is to simply move the variables outside the nested
'if' block. To keep valgrind quiet we also zero-initialize
the 'control' buffer.
Reviewed-by: Eric Blake
Signed-off-by: Daniel P. Berrange
---
io/channel-socket.c| 7 ++-
tests/test-io-channel-socket.c | 96 ++
2 files changed, 99 insertions(+), 4 deletions(-)
diff --git a/io/channel-socket.c b/io/channel-socket.c
index eed2ff5..10a5b31 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -493,15 +493,14 @@ static ssize_t qio_channel_socket_writev(QIOChannel *ioc,
QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(ioc);
ssize_t ret;
struct msghdr msg = { NULL, };
+char control[CMSG_SPACE(sizeof(int) * SOCKET_MAX_FDS)] = { 0 };
+size_t fdsize = sizeof(int) * nfds;
+struct cmsghdr *cmsg;
msg.msg_iov = (struct iovec *)iov;
msg.msg_iovlen = niov;
if (nfds) {
-char control[CMSG_SPACE(sizeof(int) * SOCKET_MAX_FDS)];
-size_t fdsize = sizeof(int) * nfds;
-struct cmsghdr *cmsg;
-
if (nfds > SOCKET_MAX_FDS) {
error_setg_errno(errp, -EINVAL,
"Only %d FDs can be sent, got %zu",
diff --git a/tests/test-io-channel-socket.c b/tests/test-io-channel-socket.c
index b0eb538..1a36a3c 100644
--- a/tests/test-io-channel-socket.c
+++ b/tests/test-io-channel-socket.c
@@ -376,6 +376,100 @@ static void test_io_channel_unix_async(void)
{
return test_io_channel_unix(true);
}
+
+static void test_io_channel_unix_fd_pass(void)
+{
+SocketAddress *listen_addr = g_new0(SocketAddress, 1);
+SocketAddress *connect_addr = g_new0(SocketAddress, 1);
+QIOChannel *src, *dst;
+int testfd;
+int fdsend[3];
+int *fdrecv = NULL;
+size_t nfdrecv = 0;
+size_t i;
+char bufsend[12], bufrecv[12];
+struct iovec iosend[1], iorecv[1];
+
+#define TEST_SOCKET "test-io-channel-socket.sock"
+#define TEST_FILE "test-io-channel-socket.txt"
+
+testfd = open(TEST_FILE, O_RDWR|O_TRUNC|O_CREAT, 0700);
+g_assert(testfd != -1);
+fdsend[0] = testfd;
+fdsend[1] = testfd;
+fdsend[2] = testfd;
+
+listen_addr->type = SOCKET_ADDRESS_KIND_UNIX;
+listen_addr->u.q_unix = g_new0(UnixSocketAddress, 1);
+listen_addr->u.q_unix->path = g_strdup(TEST_SOCKET);
+
+connect_addr->type = SOCKET_ADDRESS_KIND_UNIX;
+connect_addr->u.q_unix = g_new0(UnixSocketAddress, 1);
+connect_addr->u.q_unix->path = g_strdup(TEST_SOCKET);
+
+test_io_channel_setup_sync(listen_addr, connect_addr, &src, &dst);
+
+memcpy(bufsend, "Hello World", G_N_ELEMENTS(bufsend));
+
+iosend[0].iov_base = bufsend;
+iosend[0].iov_len = G_N_ELEMENTS(bufsend);
+
+iorecv[0].iov_base = bufrecv;
+iorecv[0].iov_len = G_N_ELEMENTS(bufrecv);
+
+g_assert(qio_channel_has_feature(src, QIO_CHANNEL_FEATURE_FD_PASS));
+g_assert(qio_channel_has_feature(dst, QIO_CHANNEL_FEATURE_FD_PASS));
+
+qio_channel_writev_full(src,
+iosend,
+G_N_ELEMENTS(iosend),
+fdsend,
+G_N_ELEMENTS(fdsend),
+&error_abort);
+
+qio_channel_readv_full(dst,
+ iorecv,
+ G_N_ELEMENTS(iorecv),
+ &fdrecv,
+ &nfdrecv,
+ &error_abort);
+
+g_assert(nfdrecv == G_N_ELEMENTS(fdsend));
+/* Each recvd FD should be different from sent FD */
+for (i = 0; i < nfdrecv; i++) {
+g_assert_cmpint(fdrecv[i], !=, testfd);
+}
+/* Each recvd FD should be different from each other */
+g_assert_cmpint(fdrecv[0], !=, fdrecv[1]);
+g_assert_cmpint(fdrecv[0], !=, fdrecv[2]);
+g_assert_cmpint(fdrecv[1], !=, fdrecv[2]);
+
+/* Check the I/O buf we sent at the same time matches */
+g_assert(memcmp(bufsend, bufrecv, G_N_ELEMENTS(bufsend)) == 0);
+
+/* Write some data into the FD we received */
+g_assert(write(fdrecv[0], bufsend, G_N_ELEMENTS(bufsend)) ==
+ G_N_ELEMENTS(bufsend));
+
+/* Read data from the original FD and make sure it matches */
+memset(bufrecv, 0, G_N_ELEMENTS(bufrecv));
+g_assert(lseek(testfd, 0, SEEK_SET) == 0);
+g_assert(read(testfd, bufrecv, G_N_ELEMENTS(bufrecv)) ==
+ G_N_ELEMENTS(bufrecv));
+g_assert(memcmp(bufsend, bufrecv, G_N_ELEMENTS(bufsend)) == 0);
+
+object_unref(OBJECT(src));
+object_unref(OBJECT(dst));
+qapi_free_SocketAddress(listen_ad
[Qemu-devel] [PULL] 9p fix
The following changes since commit 5dc42c186d63b7b338594fc071cf290805dcc5a5: Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2015-12-22 14:21:42 +) are available in the git repository at: https://github.com/gkurz/qemu.git tags/for-upstream for you to fetch changes up to 4b3a4f2d458ca5a7c6c16ac36a8d9ac22cc253d6: virtio-9p: use accessor to get thread_pool (2015-12-23 10:56:58 +0100) Fix a 2.5 regression. Greg Kurz (1): virtio-9p: use accessor to get thread_pool hw/9pfs/virtio-9p-coth.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.4.3
[Qemu-devel] [PULL] virtio-9p: use accessor to get thread_pool
The aio_context_new() function does not allocate a thread pool. This is
deferred to the first call to the aio_get_thread_pool() accessor. It is
hence forbidden to access the thread_pool field directly, as it may be
NULL. The accessor *must* be used always.
Fixes: ebac1202c95a4f1b76b6ef3f0f63926fa76e753e
Reviewed-by: Michael Tokarev
Tested-by: Michael Tokarev
Cc: qemu-sta...@nongnu.org
Signed-off-by: Greg Kurz
---
hw/9pfs/virtio-9p-coth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/9pfs/virtio-9p-coth.c b/hw/9pfs/virtio-9p-coth.c
index fb6e8f80e0f4..ab9425c60fd2 100644
--- a/hw/9pfs/virtio-9p-coth.c
+++ b/hw/9pfs/virtio-9p-coth.c
@@ -36,6 +36,6 @@ static int coroutine_enter_func(void *arg)
void co_run_in_worker_bh(void *opaque)
{
Coroutine *co = opaque;
-thread_pool_submit_aio(qemu_get_aio_context()->thread_pool,
+thread_pool_submit_aio(aio_get_thread_pool(qemu_get_aio_context()),
coroutine_enter_func, co, coroutine_enter_cb, co);
}
--
2.4.3
[Qemu-devel] [PULL v1 2/6] crypto: add ability to query hash digest len
Add a qcrypto_hash_digest_len() method which allows querying of
the raw digest size for a given hash algorithm.
Reviewed-by: Eric Blake
Signed-off-by: Daniel P. Berrange
---
crypto/hash.c| 15 +++
include/crypto/hash.h| 11 +++
tests/test-crypto-hash.c | 5 +
3 files changed, 31 insertions(+)
diff --git a/crypto/hash.c b/crypto/hash.c
index 81e74de..5a47b90 100644
--- a/crypto/hash.c
+++ b/crypto/hash.c
@@ -30,6 +30,12 @@ static int qcrypto_hash_alg_map[QCRYPTO_HASH_ALG_LAST] = {
[QCRYPTO_HASH_ALG_SHA256] = GNUTLS_DIG_SHA256,
};
+static size_t qcrypto_hash_alg_size[QCRYPTO_HASH_ALG_LAST] = {
+[QCRYPTO_HASH_ALG_MD5] = 16,
+[QCRYPTO_HASH_ALG_SHA1] = 20,
+[QCRYPTO_HASH_ALG_SHA256] = 32,
+};
+
gboolean qcrypto_hash_supports(QCryptoHashAlgorithm alg)
{
if (alg < G_N_ELEMENTS(qcrypto_hash_alg_map)) {
@@ -38,6 +44,15 @@ gboolean qcrypto_hash_supports(QCryptoHashAlgorithm alg)
return false;
}
+size_t qcrypto_hash_digest_len(QCryptoHashAlgorithm alg)
+{
+if (alg >= G_N_ELEMENTS(qcrypto_hash_alg_size)) {
+return 0;
+}
+return qcrypto_hash_alg_size[alg];
+}
+
+
int qcrypto_hash_bytesv(QCryptoHashAlgorithm alg,
const struct iovec *iov,
size_t niov,
diff --git a/include/crypto/hash.h b/include/crypto/hash.h
index b5acbf6..3d18124 100644
--- a/include/crypto/hash.h
+++ b/include/crypto/hash.h
@@ -44,6 +44,17 @@ typedef enum {
*/
gboolean qcrypto_hash_supports(QCryptoHashAlgorithm alg);
+
+/**
+ * qcrypto_hash_digest_len:
+ * @alg: the hash algorithm
+ *
+ * Determine the size of the hash digest in bytes
+ *
+ * Returns: the digest length in bytes
+ */
+size_t qcrypto_hash_digest_len(QCryptoHashAlgorithm alg);
+
/**
* qcrypto_hash_bytesv:
* @alg: the hash algorithm
diff --git a/tests/test-crypto-hash.c b/tests/test-crypto-hash.c
index 911437e..3ec31dd 100644
--- a/tests/test-crypto-hash.c
+++ b/tests/test-crypto-hash.c
@@ -163,6 +163,11 @@ static void test_hash_digest(void)
for (i = 0; i < G_N_ELEMENTS(expected_outputs) ; i++) {
int ret;
char *digest;
+size_t digestsize;
+
+digestsize = qcrypto_hash_digest_len(i);
+
+g_assert_cmpint(digestsize * 2, ==, strlen(expected_outputs[i]));
ret = qcrypto_hash_digest(i,
INPUT_TEXT,
--
2.5.0
[Qemu-devel] [PULL v1 3/6] crypto: move QCryptoHashAlgorithm enum definition into QAPI
The QCryptoHashAlgorithm enum is defined in the crypto/hash.h
header. In the future some QAPI types will want to reference
the hash enums, so move the enum definition into QAPI too.
Reviewed-by: Eric Blake
Signed-off-by: Daniel P. Berrange
---
crypto/hash.c | 4 ++--
include/crypto/hash.h | 9 +
qapi/crypto.json | 15 +++
3 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/crypto/hash.c b/crypto/hash.c
index 5a47b90..6e83f43 100644
--- a/crypto/hash.c
+++ b/crypto/hash.c
@@ -24,13 +24,13 @@
#include
#include
-static int qcrypto_hash_alg_map[QCRYPTO_HASH_ALG_LAST] = {
+static int qcrypto_hash_alg_map[QCRYPTO_HASH_ALG__MAX] = {
[QCRYPTO_HASH_ALG_MD5] = GNUTLS_DIG_MD5,
[QCRYPTO_HASH_ALG_SHA1] = GNUTLS_DIG_SHA1,
[QCRYPTO_HASH_ALG_SHA256] = GNUTLS_DIG_SHA256,
};
-static size_t qcrypto_hash_alg_size[QCRYPTO_HASH_ALG_LAST] = {
+static size_t qcrypto_hash_alg_size[QCRYPTO_HASH_ALG__MAX] = {
[QCRYPTO_HASH_ALG_MD5] = 16,
[QCRYPTO_HASH_ALG_SHA1] = 20,
[QCRYPTO_HASH_ALG_SHA256] = 32,
diff --git a/include/crypto/hash.h b/include/crypto/hash.h
index 3d18124..41822c0 100644
--- a/include/crypto/hash.h
+++ b/include/crypto/hash.h
@@ -24,14 +24,7 @@
#include "qemu-common.h"
#include "qapi/error.h"
-typedef enum {
-QCRYPTO_HASH_ALG_MD5,
-QCRYPTO_HASH_ALG_SHA1,
-QCRYPTO_HASH_ALG_SHA256,
-
-QCRYPTO_HASH_ALG_LAST
-} QCryptoHashAlgorithm;
-
+/* See also "QCryptoHashAlgorithm" defined in qapi/crypto.json */
/**
* qcrypto_hash_supports:
diff --git a/qapi/crypto.json b/qapi/crypto.json
index 4012659..0706ded 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -33,3 +33,18 @@
{ 'enum': 'QCryptoSecretFormat',
'prefix': 'QCRYPTO_SECRET_FORMAT',
'data': ['raw', 'base64']}
+
+
+##
+# QCryptoHashAlgorithm:
+#
+# The supported algorithms for computing content digests
+#
+# @md5: MD5. Should not be used in any new code, legacy compat only
+# @sha1: SHA-1. Should not be used in any new code, legacy compat only
+# @sha256: SHA-256. Current recommended strong hash.
+# Since: 2.6
+##
+{ 'enum': 'QCryptoHashAlgorithm',
+ 'prefix': 'QCRYPTO_HASH_ALG',
+ 'data': ['md5', 'sha1', 'sha256']}
--
2.5.0
[Qemu-devel] [PULL v1 0/6] Misc crypto changes & fixes
The following changes since commit 5dc42c186d63b7b338594fc071cf290805dcc5a5: Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2015-12-22 14:21:42 +) are available in the git repository at: git://github.com/berrange/qemu tags/pull-crypto-fixes-2015-12-23-1 for you to fetch changes up to 50de6261510301d586f0411b3fdb11e4cd4fdb52: crypto: fix transposed arguments in cipher error message (2015-12-23 11:02:20 +) Merge misc crypto changes & fixes Daniel P. Berrange (6): crypto: add additional query accessors for cipher instances crypto: add ability to query hash digest len crypto: move QCryptoHashAlgorithm enum definition into QAPI crypto: move QCryptoCipherAlgorithm/Mode enum definitions into QAPI crypto: ensure qapi/crypto.json is listed in qapi-modules crypto: fix transposed arguments in cipher error message Makefile | 3 ++- crypto/cipher.c| 54 +++--- crypto/hash.c | 17 ++- include/crypto/cipher.h| 54 +- include/crypto/hash.h | 20 ++--- qapi/crypto.json | 45 ++ tests/test-crypto-cipher.c | 10 + tests/test-crypto-hash.c | 5 + 8 files changed, 180 insertions(+), 28 deletions(-) -- 2.5.0
[Qemu-devel] [PULL v1 1/6] crypto: add additional query accessors for cipher instances
Adds new methods to allow querying the length of the cipher
key, block size and initialization vectors.
Reviewed-by: Eric Blake
Signed-off-by: Daniel P. Berrange
---
crypto/cipher.c| 48 ++
include/crypto/cipher.h| 37 +++
tests/test-crypto-cipher.c | 10 ++
3 files changed, 95 insertions(+)
diff --git a/crypto/cipher.c b/crypto/cipher.c
index c8bd180..d02bb32 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -28,6 +28,54 @@ static size_t alg_key_len[QCRYPTO_CIPHER_ALG_LAST] = {
[QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
};
+static size_t alg_block_len[QCRYPTO_CIPHER_ALG_LAST] = {
+[QCRYPTO_CIPHER_ALG_AES_128] = 16,
+[QCRYPTO_CIPHER_ALG_AES_192] = 16,
+[QCRYPTO_CIPHER_ALG_AES_256] = 16,
+[QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
+};
+
+static bool mode_need_iv[QCRYPTO_CIPHER_MODE_LAST] = {
+[QCRYPTO_CIPHER_MODE_ECB] = false,
+[QCRYPTO_CIPHER_MODE_CBC] = true,
+};
+
+
+size_t qcrypto_cipher_get_block_len(QCryptoCipherAlgorithm alg)
+{
+if (alg >= G_N_ELEMENTS(alg_key_len)) {
+return 0;
+}
+return alg_block_len[alg];
+}
+
+
+size_t qcrypto_cipher_get_key_len(QCryptoCipherAlgorithm alg)
+{
+if (alg >= G_N_ELEMENTS(alg_key_len)) {
+return 0;
+}
+return alg_key_len[alg];
+}
+
+
+size_t qcrypto_cipher_get_iv_len(QCryptoCipherAlgorithm alg,
+ QCryptoCipherMode mode)
+{
+if (alg >= G_N_ELEMENTS(alg_block_len)) {
+return 0;
+}
+if (mode >= G_N_ELEMENTS(mode_need_iv)) {
+return 0;
+}
+
+if (mode_need_iv[mode]) {
+return alg_block_len[alg];
+}
+return 0;
+}
+
+
static bool
qcrypto_cipher_validate_key_length(QCryptoCipherAlgorithm alg,
size_t nkey,
diff --git a/include/crypto/cipher.h b/include/crypto/cipher.h
index b4d714f..aa51c89 100644
--- a/include/crypto/cipher.h
+++ b/include/crypto/cipher.h
@@ -107,6 +107,43 @@ struct QCryptoCipher {
*/
bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg);
+/**
+ * qcrypto_cipher_get_block_len:
+ * @alg: the cipher algorithm
+ *
+ * Get the required data block size in bytes. When
+ * encrypting data, it must be a multiple of the
+ * block size.
+ *
+ * Returns: the block size in bytes
+ */
+size_t qcrypto_cipher_get_block_len(QCryptoCipherAlgorithm alg);
+
+
+/**
+ * qcrypto_cipher_get_key_len:
+ * @alg: the cipher algorithm
+ *
+ * Get the required key size in bytes.
+ *
+ * Returns: the key size in bytes
+ */
+size_t qcrypto_cipher_get_key_len(QCryptoCipherAlgorithm alg);
+
+
+/**
+ * qcrypto_cipher_get_iv_len:
+ * @alg: the cipher algorithm
+ * @mode: the cipher mode
+ *
+ * Get the required initialization vector size
+ * in bytes, if one is required.
+ *
+ * Returns: the IV size in bytes, or 0 if no IV is permitted
+ */
+size_t qcrypto_cipher_get_iv_len(QCryptoCipherAlgorithm alg,
+ QCryptoCipherMode mode);
+
/**
* qcrypto_cipher_new:
diff --git a/tests/test-crypto-cipher.c b/tests/test-crypto-cipher.c
index f4946a0..c687307 100644
--- a/tests/test-crypto-cipher.c
+++ b/tests/test-crypto-cipher.c
@@ -229,6 +229,7 @@ static void test_cipher(const void *opaque)
uint8_t *key, *iv, *ciphertext, *plaintext, *outtext;
size_t nkey, niv, nciphertext, nplaintext;
char *outtexthex;
+size_t ivsize, keysize, blocksize;
nkey = unhex_string(data->key, &key);
niv = unhex_string(data->iv, &iv);
@@ -245,6 +246,15 @@ static void test_cipher(const void *opaque)
&error_abort);
g_assert(cipher != NULL);
+keysize = qcrypto_cipher_get_key_len(data->alg);
+blocksize = qcrypto_cipher_get_block_len(data->alg);
+ivsize = qcrypto_cipher_get_iv_len(data->alg, data->mode);
+
+g_assert_cmpint(keysize, ==, nkey);
+g_assert_cmpint(ivsize, ==, niv);
+if (niv) {
+g_assert_cmpint(blocksize, ==, niv);
+}
if (iv) {
g_assert(qcrypto_cipher_setiv(cipher,
--
2.5.0
[Qemu-devel] [PULL v1 4/6] crypto: move QCryptoCipherAlgorithm/Mode enum definitions into QAPI
The QCryptoCipherAlgorithm and QCryptoCipherMode enums are
defined in the crypto/cipher.h header. In the future some
QAPI types will want to reference the hash enums, so move
the enum definition into QAPI too.
Reviewed-by: Eric Blake
Signed-off-by: Daniel P. Berrange
---
crypto/cipher.c | 8
include/crypto/cipher.h | 17 ++---
qapi/crypto.json| 30 ++
3 files changed, 36 insertions(+), 19 deletions(-)
diff --git a/crypto/cipher.c b/crypto/cipher.c
index d02bb32..a24677c 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -21,21 +21,21 @@
#include "crypto/cipher.h"
-static size_t alg_key_len[QCRYPTO_CIPHER_ALG_LAST] = {
+static size_t alg_key_len[QCRYPTO_CIPHER_ALG__MAX] = {
[QCRYPTO_CIPHER_ALG_AES_128] = 16,
[QCRYPTO_CIPHER_ALG_AES_192] = 24,
[QCRYPTO_CIPHER_ALG_AES_256] = 32,
[QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
};
-static size_t alg_block_len[QCRYPTO_CIPHER_ALG_LAST] = {
+static size_t alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {
[QCRYPTO_CIPHER_ALG_AES_128] = 16,
[QCRYPTO_CIPHER_ALG_AES_192] = 16,
[QCRYPTO_CIPHER_ALG_AES_256] = 16,
[QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
};
-static bool mode_need_iv[QCRYPTO_CIPHER_MODE_LAST] = {
+static bool mode_need_iv[QCRYPTO_CIPHER_MODE__MAX] = {
[QCRYPTO_CIPHER_MODE_ECB] = false,
[QCRYPTO_CIPHER_MODE_CBC] = true,
};
@@ -81,7 +81,7 @@ qcrypto_cipher_validate_key_length(QCryptoCipherAlgorithm alg,
size_t nkey,
Error **errp)
{
-if ((unsigned)alg >= QCRYPTO_CIPHER_ALG_LAST) {
+if ((unsigned)alg >= QCRYPTO_CIPHER_ALG__MAX) {
error_setg(errp, "Cipher algorithm %d out of range",
alg);
return false;
diff --git a/include/crypto/cipher.h b/include/crypto/cipher.h
index aa51c89..a812803 100644
--- a/include/crypto/cipher.h
+++ b/include/crypto/cipher.h
@@ -26,21 +26,8 @@
typedef struct QCryptoCipher QCryptoCipher;
-typedef enum {
-QCRYPTO_CIPHER_ALG_AES_128,
-QCRYPTO_CIPHER_ALG_AES_192,
-QCRYPTO_CIPHER_ALG_AES_256,
-QCRYPTO_CIPHER_ALG_DES_RFB, /* A stupid variant on DES for VNC */
-
-QCRYPTO_CIPHER_ALG_LAST
-} QCryptoCipherAlgorithm;
-
-typedef enum {
-QCRYPTO_CIPHER_MODE_ECB,
-QCRYPTO_CIPHER_MODE_CBC,
-
-QCRYPTO_CIPHER_MODE_LAST
-} QCryptoCipherMode;
+/* See also "QCryptoCipherAlgorithm" and "QCryptoCipherMode"
+ * enums defined in qapi/crypto.json */
/**
* QCryptoCipher:
diff --git a/qapi/crypto.json b/qapi/crypto.json
index 0706ded..4bd690f 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -48,3 +48,33 @@
{ 'enum': 'QCryptoHashAlgorithm',
'prefix': 'QCRYPTO_HASH_ALG',
'data': ['md5', 'sha1', 'sha256']}
+
+
+##
+# QCryptoCipherAlgorithm:
+#
+# The supported algorithms for content encryption ciphers
+#
+# @aes-128: AES with 128 bit / 16 byte keys
+# @aes-192: AES with 192 bit / 24 byte keys
+# @aes-256: AES with 256 bit / 32 byte keys
+# @des-rfb: RFB specific variant of single DES. Do not use except in VNC.
+# Since: 2.6
+##
+{ 'enum': 'QCryptoCipherAlgorithm',
+ 'prefix': 'QCRYPTO_CIPHER_ALG',
+ 'data': ['aes-128', 'aes-192', 'aes-256', 'des-rfb']}
+
+
+##
+# QCryptoCipherMode:
+#
+# The supported modes for content encryption ciphers
+#
+# @ecb: Electronic Code Book
+# @cbc: Cipher Block Chaining
+# Since: 2.6
+##
+{ 'enum': 'QCryptoCipherMode',
+ 'prefix': 'QCRYPTO_CIPHER_MODE',
+ 'data': ['ecb', 'cbc']}
--
2.5.0
[Qemu-devel] [PULL v1 5/6] crypto: ensure qapi/crypto.json is listed in qapi-modules
The rebuild of qapi-types.c/h is not correctly triggered when qapi/crypto.json is changed because it was missing from the list of files in the qapi-modules variable. Reviewed-by: Eric Blake Signed-off-by: Daniel P. Berrange --- Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index af3e5f1..82b2fc8 100644 --- a/Makefile +++ b/Makefile @@ -271,7 +271,8 @@ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) qapi-modules = $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi/common.json \ $(SRC_PATH)/qapi/block.json $(SRC_PATH)/qapi/block-core.json \ - $(SRC_PATH)/qapi/event.json $(SRC_PATH)/qapi/introspect.json + $(SRC_PATH)/qapi/event.json $(SRC_PATH)/qapi/introspect.json \ + $(SRC_PATH)/qapi/crypto.json qapi-types.c qapi-types.h :\ $(qapi-modules) $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) -- 2.5.0
[Qemu-devel] [PULL v1 6/6] crypto: fix transposed arguments in cipher error message
When reporting an incorrect key length for a cipher, we
mixed up the actual vs expected arguments.
Reviewed-by: Eric Blake
Signed-off-by: Daniel P. Berrange
---
crypto/cipher.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crypto/cipher.c b/crypto/cipher.c
index a24677c..7c33348 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -89,7 +89,7 @@ qcrypto_cipher_validate_key_length(QCryptoCipherAlgorithm alg,
if (alg_key_len[alg] != nkey) {
error_setg(errp, "Cipher key length %zu should be %zu",
- alg_key_len[alg], nkey);
+ nkey, alg_key_len[alg]);
return false;
}
return true;
--
2.5.0
Re: [Qemu-devel] [PATCH v2] qemu-char: add logfile facility to all chardev backends
On Tue, Dec 22, 2015 at 11:45:45AM -0700, Eric Blake wrote:
> On 12/22/2015 11:17 AM, Daniel P. Berrange wrote:
> > Typically a UNIX guest OS will log boot messages to a serial
> > port in addition to any graphical console. An admin user
> > may also wish to use the serial port for an interactive
> > console. A virtualization management system may wish to
> > collect system boot messages by logging the serial port,
> > but also wish to allow admins interactive access.
>
> [meta-review of JUST qapi decisions; code review in a separate message]
>
> >
> > Currently providing such a feature forces the mgmt app
> > to either provide 2 separate serial ports, one for
> > logging boot messages and one for interactive console
> > login, or to proxy all output via a separate service
> > that can multiplex the two needs onto one serial port.
> > While both are valid approaches, they each have their
> > own downsides. The former causes confusion and extra
> > setup work for VM admins creating disk images. The latter
> > places an extra burden to re-implement much of the QEMU
> > chardev backends logic in libvirt or even higher level
> > mgmt apps and adds extra hops in the data transfer path.
> >
> > A simpler approach that is satisfactory for many use
> > cases is to allow the QEMU chardev backends to have a
> > "logfile" property associated with them.
> >
> > $QEMU -chardev socket,host=localhost,port=9000,\
> > server=on,nowait,id-charserial0,\
> > logfile=/var/log/libvirt/qemu/test-serial0.log
> >-device isa-serial,chardev=charserial0,id=serial0
> >
> > This patch introduces a 'ChardevCommon' struct which
> > is setup as a base for all the ChardevBackend types.
> > Ideally this would be registered directly as a base
> > against ChardevBackend, rather than each type, but
> > the QAPI generator doesn't allow that since the
> > ChardevBackend is a non-discriminated union.
>
> It is possible to convert ChardevBackend into a discriminated union
> while still keeping the same QMP semantics.
>
> But where it gets interesting is what the QMP semantics should be.
>
> Right now, we have (simplifying to just two branches, for less typing):
> { 'union': 'ChardevBackend',
> 'data': { 'file': 'ChardevFile',
> 'serial': 'ChardevHostdev' } }
>
> which means we support:
>
> { "execute": "chardev-add", "arguments": {
> "id": "foo", "backend": { "type": "file",
>"data": { "out": "filename" } } } }
>
> With your addition, ChardevFile now inherits from ChardevCommon, so we gain:
>
> { "execute": "chardev-add", "arguments": {
> "id": "foo", "backend": { "type": "file",
> "data": { "logfile": "logname", "out": "filename" } } } }
Ok good that matches what I intended - namely that 'logfile'
should appear at the same dict as the rest of the backend
fields, to mirror the the fact that the C struct had all
the common fields in the same struct too.
> Re-writing the existing ChardevBackend to a semantically-identical flat
> union would be (using my shorthand syntax for anonymous base [1] and
> anonymous branch wrappers [2]):
>
> { 'enum': 'ChardevType', 'data': [ 'file', 'serial' ] }
> { 'union': 'ChardevBackend',
> 'base': { 'type': 'ChardevType' }, 'discriminator': 'type',
> 'data': { 'file': { 'data': 'ChardevFile' },
> 'serial': { 'data': 'ChardevHostdev' } } }
>
> [1] http://repo.or.cz/qemu/ericb.git/commitdiff/dbb8680b1
> [2] not yet posted to list or my git repo
>
> Note that in my conversion, 'file' is no longer directly a
> 'ChardevFile', but an anonymous type with one field named 'data' where
> _that_ field is a ChardevFile; necessary to keep the existing QMP
> nesting the same.
>
> Your proposal, then, is that the new 'logging' fields in your
> ChardevCommon should be made part of the base type of the
> 'ChardevBackend' union; which would be spelled as:
>
> { 'enum': 'ChardevType', 'data': [ 'file', 'serial' ] }
> { 'struct': 'ChardevCommon', 'data': {
> 'type': 'ChardevType', '*logfile': 'str', '*logappend': bool } }
> { 'union': 'ChardevBackend',
> 'base': 'ChardevCommon', 'discriminator': 'type',
> 'data': { 'file': { 'data': 'ChardevFile' },
> 'serial': { 'data': 'ChardevHostdev' } } }
>
> But done that way, the QMP wire form would be:
>
> { "execute": "chardev-add", "arguments": {
> "id": "foo", "backend": { "type": "file",
> "logfile": "logname", "data": { "out": "filename" } } } }
>
> Note the difference: "logfile" changes from being a child of "data" to
> being a sibling.
Ok, so that's still backwards compatible, but the 'logfile' is
appearing in an expected place IMHO.
> Hmm - now that I've typed all that, I wonder if it would make more sense
> to instead just make these parameters be siblings of "backend", by
> instead modifying:
>
> { 'command': 'chardev-add', 'data': {
> 'id': 'str', 'backend': 'ChardevBackend',
> '*logfile': 'str', '*logappend': bool } }
>
> where the QMP
[Qemu-devel] [PATCH] spapr vio: fix to incomplete QOMify
Signed-off-by: Cao jin
---
hw/ppc/spapr_vio.c | 12 +---
include/hw/ppc/spapr_vio.h | 2 +-
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index c51eb8e..46f3b8d 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -388,7 +388,7 @@ static void rtas_quiesce(PowerPCCPU *cpu, sPAPRMachineState
*spapr,
static VIOsPAPRDevice *reg_conflict(VIOsPAPRDevice *dev)
{
-VIOsPAPRBus *bus = DO_UPCAST(VIOsPAPRBus, bus, dev->qdev.parent_bus);
+VIOsPAPRBus *bus = SPAPR_VIO_BUS(dev->qdev.parent_bus);
BusChild *kid;
VIOsPAPRDevice *other;
@@ -449,7 +449,7 @@ static void spapr_vio_busdev_realize(DeviceState *qdev,
Error **errp)
}
} else {
/* Need to assign an address */
-VIOsPAPRBus *bus = DO_UPCAST(VIOsPAPRBus, bus, dev->qdev.parent_bus);
+VIOsPAPRBus *bus = SPAPR_VIO_BUS(dev->qdev.parent_bus);
do {
dev->reg = bus->next_reg++;
@@ -523,13 +523,12 @@ VIOsPAPRBus *spapr_vio_bus_init(void)
DeviceState *dev;
/* Create bridge device */
-dev = qdev_create(NULL, "spapr-vio-bridge");
+dev = qdev_create(NULL, TYPE_SPAPR_VIO_BRIDGE);
qdev_init_nofail(dev);
/* Create bus on bridge device */
-
qbus = qbus_create(TYPE_SPAPR_VIO_BUS, dev, "spapr-vio");
-bus = DO_UPCAST(VIOsPAPRBus, bus, qbus);
+bus = SPAPR_VIO_BUS(qbus);
bus->next_reg = 0x7100;
/* hcall-vio */
@@ -567,9 +566,8 @@ static void spapr_vio_bridge_class_init(ObjectClass *klass,
void *data)
}
static const TypeInfo spapr_vio_bridge_info = {
-.name = "spapr-vio-bridge",
+.name = TYPE_SPAPR_VIO_BRIDGE,
.parent= TYPE_SYS_BUS_DEVICE,
-.instance_size = sizeof(SysBusDevice),
.class_init= spapr_vio_bridge_class_init,
};
diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h
index 2299a54..c9733e7 100644
--- a/include/hw/ppc/spapr_vio.h
+++ b/include/hw/ppc/spapr_vio.h
@@ -34,7 +34,7 @@
#define TYPE_SPAPR_VIO_BUS "spapr-vio-bus"
#define SPAPR_VIO_BUS(obj) OBJECT_CHECK(VIOsPAPRBus, (obj), TYPE_SPAPR_VIO_BUS)
-struct VIOsPAPRDevice;
+#define TYPE_SPAPR_VIO_BRIDGE "spapr-vio-bridge"
typedef struct VIOsPAPR_CRQ {
uint64_t qladdr;
--
2.1.0
[Qemu-devel] [PATCH v1 1/6] kvm/x86: Drop stimer_stop() function
The function stimer_stop() is called in one place
so remove the function and replace it's call by function
content.
Signed-off-by: Andrey Smetanin
Reviewed-by: Roman Kagan
CC: Gleb Natapov
CC: Paolo Bonzini
CC: Roman Kagan
CC: Denis V. Lunev
CC: qemu-devel@nongnu.org
---
arch/x86/kvm/hyperv.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index f34f666..ec3a900 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -400,16 +400,11 @@ static void stimer_mark_expired(struct kvm_vcpu_hv_stimer
*stimer,
kvm_vcpu_kick(vcpu);
}
-static void stimer_stop(struct kvm_vcpu_hv_stimer *stimer)
-{
- hrtimer_cancel(&stimer->timer);
-}
-
static void stimer_cleanup(struct kvm_vcpu_hv_stimer *stimer)
{
struct kvm_vcpu *vcpu = stimer_to_vcpu(stimer);
- stimer_stop(stimer);
+ hrtimer_cancel(&stimer->timer);
clear_bit(stimer->index,
vcpu_to_hv_vcpu(vcpu)->stimer_pending_bitmap);
stimer->msg_pending = false;
--
2.4.3
[Qemu-devel] [PATCH v1 4/6] kvm/x86: Hyper-V fix SynIC timer disabling condition
Hypervisor Function Specification(HFS) doesn't require
to disable SynIC timer at timer config write if timer->count = 0.
So drop this check, this allow to load timers MSR's
during migration restore, because config are set before count
in QEMU side.
Also fix condition according to HFS doc(15.3.1):
"It is not permitted to set the SINTx field to zero for an
enabled timer. If attempted, the timer will be
marked disabled (that is, bit 0 cleared) immediately."
Signed-off-by: Andrey Smetanin
Reviewed-by: Roman Kagan
CC: Gleb Natapov
CC: Paolo Bonzini
CC: Roman Kagan
CC: Denis V. Lunev
CC: qemu-devel@nongnu.org
---
arch/x86/kvm/hyperv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index ce17529..b203ce3 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -472,7 +472,7 @@ static int stimer_start(struct kvm_vcpu_hv_stimer *stimer)
static int stimer_set_config(struct kvm_vcpu_hv_stimer *stimer, u64 config,
bool host)
{
- if (stimer->count == 0 || HV_STIMER_SINT(config) == 0)
+ if ((stimer->config & HV_STIMER_ENABLE) && HV_STIMER_SINT(config) == 0)
config &= ~HV_STIMER_ENABLE;
stimer->config = config;
stimer_cleanup(stimer);
--
2.4.3
[Qemu-devel] [PATCH v1 3/6] kvm/x86: Reorg stimer_expiration() to better control timer restart
Split stimer_expiration() into two parts - timer expiration message
sending and timer restart/cleanup based on timer state(config).
This also fixes a bug where a one-shot timer message whose delivery
failed once would get lost for good.
Signed-off-by: Andrey Smetanin
Reviewed-by: Roman Kagan
CC: Gleb Natapov
CC: Paolo Bonzini
CC: Roman Kagan
CC: Denis V. Lunev
CC: qemu-devel@nongnu.org
---
arch/x86/kvm/hyperv.c | 26 ++
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 8623aa6..ce17529 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -552,30 +552,27 @@ static int synic_deliver_msg(struct kvm_vcpu_hv_synic
*synic, u32 sint,
return r;
}
-static void stimer_send_msg(struct kvm_vcpu_hv_stimer *stimer)
+static int stimer_send_msg(struct kvm_vcpu_hv_stimer *stimer)
{
struct kvm_vcpu *vcpu = stimer_to_vcpu(stimer);
struct hv_message *msg = &stimer->msg;
struct hv_timer_message_payload *payload =
(struct hv_timer_message_payload *)&msg->u.payload;
- int r;
- stimer->msg_pending = true;
payload->expiration_time = stimer->exp_time;
payload->delivery_time = get_time_ref_counter(vcpu->kvm);
- r = synic_deliver_msg(vcpu_to_synic(vcpu),
- HV_STIMER_SINT(stimer->config), msg);
- if (!r)
- stimer->msg_pending = false;
+ return synic_deliver_msg(vcpu_to_synic(vcpu),
+HV_STIMER_SINT(stimer->config), msg);
}
static void stimer_expiration(struct kvm_vcpu_hv_stimer *stimer)
{
- stimer_send_msg(stimer);
- if (!(stimer->config & HV_STIMER_PERIODIC))
- stimer->config |= ~HV_STIMER_ENABLE;
- else
- stimer_start(stimer);
+ stimer->msg_pending = true;
+ if (!stimer_send_msg(stimer)) {
+ stimer->msg_pending = false;
+ if (!(stimer->config & HV_STIMER_PERIODIC))
+ stimer->config |= ~HV_STIMER_ENABLE;
+ }
}
void kvm_hv_process_stimers(struct kvm_vcpu *vcpu)
@@ -592,6 +589,11 @@ void kvm_hv_process_stimers(struct kvm_vcpu *vcpu)
time_now = get_time_ref_counter(vcpu->kvm);
if (time_now >= stimer->exp_time)
stimer_expiration(stimer);
+
+ if (stimer->config & HV_STIMER_ENABLE)
+ stimer_start(stimer);
+ else
+ stimer_cleanup(stimer);
}
}
}
--
2.4.3
[Qemu-devel] [PATCH v1 6/6] kvm/x86: Update SynIC timers on guest entry only
Consolidate updating the Hyper-V SynIC timers in a
single place: on guest entry in processing KVM_REQ_HV_STIMER
request. This simplifies the overall logic, and makes sure
the most current state of msrs and guest clock is used for
arming the timers (to achieve that, KVM_REQ_HV_STIMER
has to be processed after KVM_REQ_CLOCK_UPDATE).
Signed-off-by: Andrey Smetanin
Reviewed-by: Roman Kagan
CC: Gleb Natapov
CC: Paolo Bonzini
CC: Roman Kagan
CC: Denis V. Lunev
CC: qemu-devel@nongnu.org
---
arch/x86/kvm/hyperv.c | 35 +--
arch/x86/kvm/x86.c| 6 ++
2 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index d7e6651..7857329 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -389,7 +389,7 @@ static u64 get_time_ref_counter(struct kvm *kvm)
return div_u64(get_kernel_ns() + kvm->arch.kvmclock_offset, 100);
}
-static void stimer_mark_expired(struct kvm_vcpu_hv_stimer *stimer,
+static void stimer_mark_pending(struct kvm_vcpu_hv_stimer *stimer,
bool vcpu_kick)
{
struct kvm_vcpu *vcpu = stimer_to_vcpu(stimer);
@@ -417,7 +417,7 @@ static enum hrtimer_restart stimer_timer_callback(struct
hrtimer *timer)
struct kvm_vcpu_hv_stimer *stimer;
stimer = container_of(timer, struct kvm_vcpu_hv_stimer, timer);
- stimer_mark_expired(stimer, true);
+ stimer_mark_pending(stimer, true);
return HRTIMER_NORESTART;
}
@@ -460,7 +460,7 @@ static int stimer_start(struct kvm_vcpu_hv_stimer *stimer)
* "If a one shot is enabled and the specified count is in
* the past, it will expire immediately."
*/
- stimer_mark_expired(stimer, false);
+ stimer_mark_pending(stimer, false);
return 0;
}
@@ -473,30 +473,24 @@ static int stimer_start(struct kvm_vcpu_hv_stimer *stimer)
static int stimer_set_config(struct kvm_vcpu_hv_stimer *stimer, u64 config,
bool host)
{
+ stimer_cleanup(stimer);
if ((stimer->config & HV_STIMER_ENABLE) && HV_STIMER_SINT(config) == 0)
config &= ~HV_STIMER_ENABLE;
stimer->config = config;
- stimer_cleanup(stimer);
- if (stimer->config & HV_STIMER_ENABLE)
- if (stimer_start(stimer))
- return 1;
+ stimer_mark_pending(stimer, false);
return 0;
}
static int stimer_set_count(struct kvm_vcpu_hv_stimer *stimer, u64 count,
bool host)
{
- stimer->count = count;
-
stimer_cleanup(stimer);
+ stimer->count = count;
if (stimer->count == 0)
stimer->config &= ~HV_STIMER_ENABLE;
- else if (stimer->config & HV_STIMER_AUTOENABLE) {
+ else if (stimer->config & HV_STIMER_AUTOENABLE)
stimer->config |= HV_STIMER_ENABLE;
- if (stimer_start(stimer))
- return 1;
- }
-
+ stimer_mark_pending(stimer, false);
return 0;
}
@@ -580,16 +574,21 @@ void kvm_hv_process_stimers(struct kvm_vcpu *vcpu)
{
struct kvm_vcpu_hv *hv_vcpu = vcpu_to_hv_vcpu(vcpu);
struct kvm_vcpu_hv_stimer *stimer;
- u64 time_now;
+ u64 time_now, exp_time;
int i;
for (i = 0; i < ARRAY_SIZE(hv_vcpu->stimer); i++)
if (test_and_clear_bit(i, hv_vcpu->stimer_pending_bitmap)) {
stimer = &hv_vcpu->stimer[i];
if (stimer->config & HV_STIMER_ENABLE) {
- time_now = get_time_ref_counter(vcpu->kvm);
- if (time_now >= stimer->exp_time)
- stimer_expiration(stimer);
+ exp_time = stimer->exp_time;
+
+ if (exp_time) {
+ time_now =
+ get_time_ref_counter(vcpu->kvm);
+ if (time_now >= exp_time)
+ stimer_expiration(stimer);
+ }
if (stimer->config & HV_STIMER_ENABLE)
stimer_start(stimer);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index b6102c1..795c14c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6492,6 +6492,12 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
r = 0;
goto out;
}
+
+ /*
+* KVM_REQ_HV_STIMER has to be processed after
+* KVM_REQ_CLOCK_UPDATE, because Hyper-V SynIC timers
+* depend on the guest clock being up-to-date
+*/
if (kvm_check_request(KVM_REQ_HV_STIMER, vcpu))
[Qemu-devel] [PATCH v1 0/6] KVM: Hyper-V SynIC timers migration fixes
During testing of Windows 2012R2 guest migration with Hyper-V SynIC timers enabled we found several bugs which lead to restoring guest in a hung state. This patch series provides several fixes to make the migration of guest with Hyper-V SynIC timers enabled succeed. The series applies on top of 'kvm/x86: Remove Hyper-V SynIC timer stopping' previously sent. Signed-off-by: Andrey Smetanin Reviewed-by: Roman Kagan CC: Gleb Natapov CC: Paolo Bonzini CC: Roman Kagan CC: Denis V. Lunev CC: qemu-devel@nongnu.org Andrey Smetanin (6): kvm/x86: Drop stimer_stop() function kvm/x86: Hyper-V unify stimer_start() and stimer_restart() kvm/x86: Reorg stimer_expiration() to better control timer restart kvm/x86: Hyper-V fix SynIC timer disabling condition kvm/x86: Skip SynIC vector check for QEMU side kvm/x86: Update SynIC timers on guest entry only arch/x86/kvm/hyperv.c | 112 +++--- arch/x86/kvm/x86.c| 6 +++ 2 files changed, 58 insertions(+), 60 deletions(-) -- 2.4.3
[Qemu-devel] [PATCH v1 2/6] kvm/x86: Hyper-V unify stimer_start() and stimer_restart()
This will be used in future to start Hyper-V SynIC timer
in several places by one logic in one function.
Signed-off-by: Andrey Smetanin
Reviewed-by: Roman Kagan
CC: Gleb Natapov
CC: Paolo Bonzini
CC: Roman Kagan
CC: Denis V. Lunev
CC: qemu-devel@nongnu.org
---
arch/x86/kvm/hyperv.c | 37 -
1 file changed, 16 insertions(+), 21 deletions(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index ec3a900..8623aa6 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -408,6 +408,7 @@ static void stimer_cleanup(struct kvm_vcpu_hv_stimer
*stimer)
clear_bit(stimer->index,
vcpu_to_hv_vcpu(vcpu)->stimer_pending_bitmap);
stimer->msg_pending = false;
+ stimer->exp_time = 0;
}
static enum hrtimer_restart stimer_timer_callback(struct hrtimer *timer)
@@ -420,24 +421,6 @@ static enum hrtimer_restart stimer_timer_callback(struct
hrtimer *timer)
return HRTIMER_NORESTART;
}
-static void stimer_restart(struct kvm_vcpu_hv_stimer *stimer)
-{
- u64 time_now;
- ktime_t ktime_now;
- u64 remainder;
-
- time_now = get_time_ref_counter(stimer_to_vcpu(stimer)->kvm);
- ktime_now = ktime_get();
-
- div64_u64_rem(time_now - stimer->exp_time, stimer->count, &remainder);
- stimer->exp_time = time_now + (stimer->count - remainder);
-
- hrtimer_start(&stimer->timer,
- ktime_add_ns(ktime_now,
- 100 * (stimer->exp_time - time_now)),
- HRTIMER_MODE_ABS);
-}
-
static int stimer_start(struct kvm_vcpu_hv_stimer *stimer)
{
u64 time_now;
@@ -450,9 +433,21 @@ static int stimer_start(struct kvm_vcpu_hv_stimer *stimer)
if (stimer->count == 0)
return -EINVAL;
- stimer->exp_time = time_now + stimer->count;
+ if (stimer->exp_time) {
+ if (time_now >= stimer->exp_time) {
+ u64 remainder;
+
+ div64_u64_rem(time_now - stimer->exp_time,
+ stimer->count, &remainder);
+ stimer->exp_time =
+ time_now + (stimer->count - remainder);
+ }
+ } else
+ stimer->exp_time = time_now + stimer->count;
+
hrtimer_start(&stimer->timer,
- ktime_add_ns(ktime_now, 100 * stimer->count),
+ ktime_add_ns(ktime_now,
+ 100 * (stimer->exp_time - time_now)),
HRTIMER_MODE_ABS);
return 0;
}
@@ -580,7 +575,7 @@ static void stimer_expiration(struct kvm_vcpu_hv_stimer
*stimer)
if (!(stimer->config & HV_STIMER_PERIODIC))
stimer->config |= ~HV_STIMER_ENABLE;
else
- stimer_restart(stimer);
+ stimer_start(stimer);
}
void kvm_hv_process_stimers(struct kvm_vcpu *vcpu)
--
2.4.3
[Qemu-devel] [PATCH v1 5/6] kvm/x86: Skip SynIC vector check for QEMU side
QEMU zero-inits Hyper-V SynIC vectors. We should allow that,
and don't reject zero values if set by the host.
Signed-off-by: Andrey Smetanin
Reviewed-by: Roman Kagan
CC: Gleb Natapov
CC: Paolo Bonzini
CC: Roman Kagan
CC: Denis V. Lunev
CC: qemu-devel@nongnu.org
---
arch/x86/kvm/hyperv.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index b203ce3..d7e6651 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -72,12 +72,13 @@ static bool synic_has_vector_auto_eoi(struct
kvm_vcpu_hv_synic *synic,
return false;
}
-static int synic_set_sint(struct kvm_vcpu_hv_synic *synic, int sint, u64 data)
+static int synic_set_sint(struct kvm_vcpu_hv_synic *synic, int sint,
+ u64 data, bool host)
{
int vector;
vector = data & HV_SYNIC_SINT_VECTOR_MASK;
- if (vector < 16)
+ if (vector < 16 && !host)
return 1;
/*
* Guest may configure multiple SINTs to use the same vector, so
@@ -247,7 +248,7 @@ static int synic_set_msr(struct kvm_vcpu_hv_synic *synic,
break;
}
case HV_X64_MSR_SINT0 ... HV_X64_MSR_SINT15:
- ret = synic_set_sint(synic, msr - HV_X64_MSR_SINT0, data);
+ ret = synic_set_sint(synic, msr - HV_X64_MSR_SINT0, data, host);
break;
default:
ret = 1;
--
2.4.3
Re: [Qemu-devel] [PATCH v2] qemu-char: add logfile facility to all chardev backends
On Tue, Dec 22, 2015 at 12:07:03PM -0700, Eric Blake wrote:
> On 12/22/2015 11:17 AM, Daniel P. Berrange wrote:
> > Typically a UNIX guest OS will log boot messages to a serial
> > port in addition to any graphical console. An admin user
> > may also wish to use the serial port for an interactive
> > console. A virtualization management system may wish to
> > collect system boot messages by logging the serial port,
> > but also wish to allow admins interactive access.
> >
>
> [code review, if we go with this design; see my other message for 2
> possible alternative qapi designs that may supersede this code review]
>
> > A simpler approach that is satisfactory for many use
> > cases is to allow the QEMU chardev backends to have a
> > "logfile" property associated with them.
> >
> > $QEMU -chardev socket,host=localhost,port=9000,\
> > server=on,nowait,id-charserial0,\
> > logfile=/var/log/libvirt/qemu/test-serial0.log
> >-device isa-serial,chardev=charserial0,id=serial0
> >
> > This patch introduces a 'ChardevCommon' struct which
> > is setup as a base for all the ChardevBackend types.
> > Ideally this would be registered directly as a base
> > against ChardevBackend, rather than each type, but
> > the QAPI generator doesn't allow that since the
> > ChardevBackend is a non-discriminated union. The
> > ChardevCommon struct provides the optional 'logfile'
> > parameter, as well as 'logappend' which controls
> > whether QEMU truncates or appends (default truncate).
> >
> > Signed-off-by: Daniel P. Berrange
> > ---
> >
>
> > +++ b/qapi-schema.json
> > @@ -3093,6 +3093,21 @@
> > ##
> > { 'command': 'screendump', 'data': {'filename': 'str'} }
> >
> > +
> > +##
> > +# @ChardevCommon:
> > +#
> > +# Configuration shared across all chardev backends
> > +#
> > +# @logfile: #optional The name of a logfile to save output
> > +# @logappend: #optional true to append instead of truncate
> > +# (default to false to truncate)
>
> Could shorten to:
>
> # @logappend: #optional true to append to @logfile (default false to
> truncate)
>
> > ##
> > # @ChardevBackend:
> > @@ -3243,7 +3269,8 @@
> > #
> > # Since: 1.4 (testdev since 2.2)
> > ##
> > -{ 'struct': 'ChardevDummy', 'data': { } }
> > +{ 'struct': 'ChardevDummy', 'data': { },
> > + 'base': 'ChardevCommon' }
>
> Instead of changing ChardevDummy, you could have deleted this type and done:
>
> >
> > { 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile',
> > 'serial' : 'ChardevHostdev',
> ...
> 'pty': 'ChardevCommon',
> 'null': 'ChardevCommon',
>
> and so on. I don't know which is better.
Yep, me neither. Given the name it felt like some kind of placeholder
for future work, so I left it alone.
> > +/* Not reporting errors from writing to logfile, as logs are
> > + * defined to be "best effort" only */
> > +static void qemu_chr_fe_write_log(CharDriverState *s,
> > + const uint8_t *buf, size_t len)
> > +{
> > +size_t done = 0;
> > +ssize_t ret;
> > +
> > +if (s->logfd < 0) {
> > +return;
> > +}
> > +
> > +while (done < len) {
> > +do {
> > +ret = write(s->logfd, buf + done, len - done);
> > +if (ret == -1 && errno == EAGAIN) {
> > +g_usleep(100);
> > +}
>
> Do we really want to be sleeping here?
If logfile points to a plain file, you'll never get EAGAIN.
For that matter even if it doesn't point to a plain file
I realize that we've not called qemu_nonblock() on logfd,
so it'll never get EAGAIN for that reason either.
The qemu_chr_fe_write_all() currently has the same usleep
which is what I followed. The qemu_chr_fe_write() just
returns on EAGAIN. In the write_log() method we want to
try to write all the data the qemu_chr_fe_write/write_all
just sent. If we ignore EGAIN, we'll potentially loose
data from the log, but if we honour it, we have to sleep
a little or not set O_NONBLOCK.
>
> > +} while (ret == -1 && errno == EAGAIN);
> > +
> > +if (ret <= 0) {
>
> Why '<='? Shouldn't this be '<'?
Well there's no difference really since write() will never
return 0.
>
> > +return;
> > +}
> > +done += ret;
> > +}
> > +}
> > +
>
> >
> > +
> > +static int qemu_chr_open_common(CharDriverState *chr,
> > +ChardevBackend *backend,
> > +Error **errp)
> > +{
> > +ChardevCommon *common = backend->u.data;
>
> Phooey. This conflicts with my pending patches to get rid of 'data':
>
> http://repo.or.cz/qemu/ericb.git/commitdiff/84aaab99
>
> I mentioned above that you could do things like 'null':'ChardevCommon'
> instead of 'null':'ChardevDummy'; and we also know that qapi guarantees
> a layout where all base types occur at the front of the res
Re: [Qemu-devel] [PATCH v2] qemu-char: add logfile facility to all chardev backends
On 22/12/2015 19:17, Daniel P. Berrange wrote:
> +if (common->has_logfile) {
> +int flags = O_WRONLY | O_CREAT;
> +if (!common->has_logappend ||
> +!common->logappend) {
> +flags |= O_TRUNC;
> +}
Should it use O_APPEND if logappend is absent or true, or perhaps
always? I can take care of the change myself.
You are also missing a few qemu_chr_alloc calls outside qemu-char.c,
which makes me wonder if it's better to add the new logic in
qemu_chr_alloc itself.
Paolo
> +chr->logfd = qemu_open(common->logfile, flags, 0666);
> +if (chr->logfd < 0) {
> +error_setg_errno(errp, errno,
> + "Unable to open logfile %s",
> + common->logfile);
> +return -1;
> +}
> +}
> +
> +return 0;
> +}
Re: [Qemu-devel] [PATCH v2] qemu-char: add logfile facility to all chardev backends
On Wed, Dec 23, 2015 at 12:34:25PM +0100, Paolo Bonzini wrote:
>
>
> On 22/12/2015 19:17, Daniel P. Berrange wrote:
> > +if (common->has_logfile) {
> > +int flags = O_WRONLY | O_CREAT;
> > +if (!common->has_logappend ||
> > +!common->logappend) {
> > +flags |= O_TRUNC;
> > +}
>
> Should it use O_APPEND if logappend is absent or true, or perhaps
> always? I can take care of the change myself.
Yes it should use O_APPEND in the other branch of the if,
that's a bug eric pointed out too. Basically same logic
as the recently added 'append' flag in qmp_chardev_open_file
> You are also missing a few qemu_chr_alloc calls outside qemu-char.c,
> which makes me wonder if it's better to add the new logic in
> qemu_chr_alloc itself.
Oh, yes, perhaps I should just pass ChardevCommon straight
into qemu_chr_alloc(). I'll look at that when doing the other
changes Eric suggested wrt to removing use of backend.u.data
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH v14 Resend 10/13] pci: add pci device pre-post reset callbacks for host bus reset
On Thu, Dec 17, 2015 at 09:41:51AM +0800, Cao jin wrote:
> From: Chen Fan
>
> Particularly, For vfio devices, Once need to recovery devices
> by bus reset such as AER, we always need to reset the host bus
> to recovery the devices under the bus, so we need to add pci device
> callbacks to specify to do host bus reset.
>
> Signed-off-by: Chen Fan
> Reviewed-by: Michael S. Tsirkin
> ---
> hw/pci/pci.c | 18 ++
> hw/pci/pci_bridge.c | 9 +
> hw/vfio/pci.c| 26 ++
> hw/vfio/pci.h| 2 ++
> include/hw/pci/pci.h | 7 +++
> 5 files changed, 62 insertions(+)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index f6ca6ef..64fa2cc 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -247,6 +247,24 @@ static void pci_do_device_reset(PCIDevice *dev)
> msix_reset(dev);
> }
>
> +void pci_device_pre_reset(PCIBus *bus, PCIDevice *dev, void *unused)
> +{
> +PCIDeviceClass *dc = PCI_DEVICE_GET_CLASS(dev);
> +
> +if (dc->pre_reset) {
> +dc->pre_reset(dev);
> +}
> +}
> +
> +void pci_device_post_reset(PCIBus *bus, PCIDevice *dev, void *unused)
> +{
> +PCIDeviceClass *dc = PCI_DEVICE_GET_CLASS(dev);
> +
> +if (dc->post_reset) {
> +dc->post_reset(dev);
> +}
> +}
> +
> /*
> * This function is called on #RST and FLR.
> * FLR if PCI_EXP_DEVCTL_BCR_FLR is set
> diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
> index 40c97b1..ddb76ab 100644
> --- a/hw/pci/pci_bridge.c
> +++ b/hw/pci/pci_bridge.c
> @@ -267,8 +267,17 @@ void pci_bridge_write_config(PCIDevice *d,
>
> newctl = pci_get_word(d->config + PCI_BRIDGE_CONTROL);
> if (~oldctl & newctl & PCI_BRIDGE_CTL_BUS_RESET) {
> +/*
> + * Notify all vfio-pci devices under the bus
> + * should do physical bus reset.
> + */
> +PCIBus *sec_bus = pci_bridge_get_sec_bus(s);
> +pci_for_each_device(sec_bus, pci_bus_num(sec_bus),
> +pci_device_pre_reset, NULL);
> /* Trigger hot reset on 0->1 transition. */
> qbus_reset_all(&s->sec_bus.qbus);
> +pci_for_each_device(sec_bus, pci_bus_num(sec_bus),
> +pci_device_post_reset, NULL);
> }
> }
>
So this boils down to need to detect hot reset,
you don't actually call this for all resets.
Callback name should reflect this IMO.
Also, why do we need two callbacks?
How about we have a single hot_reset callback,
and then if device has it, hot reset does not
invoke the regular reset?
Finally, you discussed multiple resets triggering with many vfio devices
on the same bus. To solve this, will it help to have a bus-level
callback instead?
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index e17dc89..df32618 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -39,6 +39,7 @@
>
> static void vfio_disable_interrupts(VFIOPCIDevice *vdev);
> static void vfio_mmap_set_enabled(VFIOPCIDevice *vdev, bool enabled);
> +static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single);
>
> /*
> * Disabling BAR mmaping can be slow, but toggling it around INTx can
> @@ -1888,6 +1889,8 @@ static int vfio_check_host_bus_reset(VFIOPCIDevice
> *vdev)
> /* List all affected devices by bus reset */
> devices = &info->devices[0];
>
> +vdev->single_depend_dev = (info->count == 1);
> +
> /* Verify that we have all the groups required */
> for (i = 0; i < info->count; i++) {
> PCIHostDeviceAddress host;
> @@ -2029,10 +2032,26 @@ static int vfio_check_bus_reset(NotifierWithReturn
> *n, void *opaque)
> return vfio_check_host_bus_reset(vdev);
> }
>
> +static void vfio_aer_pre_reset(PCIDevice *pdev)
> +{
> +VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
> +
> +vdev->aer_reset = true;
> +vfio_pci_hot_reset(vdev, vdev->single_depend_dev);
> +}
> +
> +static void vfio_aer_post_reset(PCIDevice *pdev)
> +{
> +VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
> +
> +vdev->aer_reset = false;
> +}
> +
> static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver,
>int pos, uint16_t size)
> {
> PCIDevice *pdev = &vdev->pdev;
> +PCIDeviceClass *dc = PCI_DEVICE_GET_CLASS(pdev);
> PCIDevice *dev_iter;
> uint8_t type;
> uint32_t errcap;
> @@ -2079,6 +2098,9 @@ static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t
> cap_ver,
> vdev->hotplug_notifier.notify = vfio_check_bus_reset;
> pci_bus_add_hotplug_notifier(pdev->bus, &vdev->hotplug_notifier);
>
> +dc->pre_reset = vfio_aer_pre_reset;
> +dc->post_reset = vfio_aer_post_reset;
> +
> pcie_cap_deverr_init(pdev);
> return pcie_aer_init(pdev, pos, size);
>
> @@ -2978,6 +3000,10 @@ static void vfio_pci_reset(DeviceState *dev)
>
> trace_vfio_pci_reset(vdev->vbasedev.name);
>
> +if (vdev->aer_reset) {
> +return;
> +}
> +
> vfio_pci_pre_reset(vdev);
>
>
Re: [Qemu-devel] [PATCH] Xen PCI passthrough: convert to realize()
On Wed, 23 Dec 2015, Cao jin wrote:
> Signed-off-by: Cao jin
> ---
>
> Since the callchain is pretty deep & error path is very much too, so I made
> the
> patch based on the principal: catch/report the most necessary error msg with
> smallest modification.(So you can see I don`t change some functions to void,
> despite they coule be)
Thanks Cao.
For consistency with the other functions, I think it would be better to
change all functions to return void or none.
Also it might be nice to split the patch in a series.
The patch as is fails to build:
qemu/hw/xen/xen_pt_config_init.c: In function ‘xen_pt_config_init’:
qemu/hw/xen/xen_pt_config_init.c:2061:42: error: ‘rc’ may be used uninitialized
in this func
> hw/xen/xen-host-pci-device.c | 79
> +++-
> hw/xen/xen-host-pci-device.h | 5 +--
> hw/xen/xen_pt.c | 67 +++--
> hw/xen/xen_pt.h | 5 +--
> hw/xen/xen_pt_config_init.c | 47 +-
> hw/xen/xen_pt_graphics.c | 6 ++--
> 6 files changed, 118 insertions(+), 91 deletions(-)
>
> diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
> index 7d8a023..1ab6d97 100644
> --- a/hw/xen/xen-host-pci-device.c
> +++ b/hw/xen/xen-host-pci-device.c
> @@ -43,13 +43,14 @@ static int xen_host_pci_sysfs_path(const XenHostPCIDevice
> *d,
> /* The output is truncated, or some other error was encountered */
> return -ENODEV;
> }
> +
> return 0;
> }
I would prefer to keep stylistic changes separate, especially the ones
in functions which would be otherwise left unmodified. Maybe you could
move them to a separate patch?
> /* This size should be enough to read the first 7 lines of a resource file */
> #define XEN_HOST_PCI_RESOURCE_BUFFER_SIZE 400
> -static int xen_host_pci_get_resource(XenHostPCIDevice *d)
> +static void xen_host_pci_get_resource(XenHostPCIDevice *d, Error **errp)
> {
> int i, rc, fd;
> char path[PATH_MAX];
> @@ -60,23 +61,24 @@ static int xen_host_pci_get_resource(XenHostPCIDevice *d)
>
> rc = xen_host_pci_sysfs_path(d, "resource", path, sizeof (path));
> if (rc) {
> -return rc;
> +error_setg_errno(errp, errno, "snprintf err");
> +return;
> }
> +
> fd = open(path, O_RDONLY);
> if (fd == -1) {
> -XEN_HOST_PCI_LOG("Error: Can't open %s: %s\n", path,
> strerror(errno));
> -return -errno;
> +error_setg_errno(errp, errno, "open err");
> +return;
> }
>
> do {
> rc = read(fd, &buf, sizeof (buf) - 1);
> if (rc < 0 && errno != EINTR) {
> -rc = -errno;
> +error_setg_errno(errp, errno, "read err");
> goto out;
> }
> } while (rc < 0);
> buf[rc] = 0;
> -rc = 0;
>
> s = buf;
> for (i = 0; i < PCI_NUM_REGIONS; i++) {
> @@ -129,20 +131,20 @@ static int xen_host_pci_get_resource(XenHostPCIDevice
> *d)
> d->rom.bus_flags = flags & IORESOURCE_BITS;
> }
> }
> +
> if (i != PCI_NUM_REGIONS) {
> /* Invalid format or input to short */
> -rc = -ENODEV;
> +error_setg(errp, "Invalid format or input to short");
^too short
> }
>
> out:
> close(fd);
> -return rc;
> }
>
> /* This size should be enough to read a long from a file */
> #define XEN_HOST_PCI_GET_VALUE_BUFFER_SIZE 22
> static int xen_host_pci_get_value(XenHostPCIDevice *d, const char *name,
> - unsigned int *pvalue, int base)
> + unsigned int *pvalue, int base, Error
> **errp)
As mentioned above, I would prefer if you could change this function to
return void too. Otherwise keep the return int everywhere.
> {
> char path[PATH_MAX];
> char buf[XEN_HOST_PCI_GET_VALUE_BUFFER_SIZE];
> @@ -152,30 +154,38 @@ static int xen_host_pci_get_value(XenHostPCIDevice *d,
> const char *name,
>
> rc = xen_host_pci_sysfs_path(d, name, path, sizeof (path));
> if (rc) {
> +error_setg_errno(errp, errno, "snprintf err");
> return rc;
> }
> +
> fd = open(path, O_RDONLY);
> if (fd == -1) {
> -XEN_HOST_PCI_LOG("Error: Can't open %s: %s\n", path,
> strerror(errno));
> +error_setg_errno(errp, errno, "open err");
would it be possible to keep the path in the error message?
> return -errno;
> }
> +
> do {
> rc = read(fd, &buf, sizeof (buf) - 1);
> if (rc < 0 && errno != EINTR) {
> +error_setg_errno(errp, errno, "read err");
> rc = -errno;
> goto out;
> }
> } while (rc < 0);
> +
> buf[rc] = 0;
> value = strtol(buf, &endptr, base);
> if (endptr == buf || *endptr != '\n') {
> +error_setg(errp, "format invalid");
> rc = -1;
> } els
Re: [Qemu-devel] [Qemu-block] [PATCH 00/10] qcow2: Implement image locking
On Wed, Dec 23, 2015 at 10:47:22AM +, Daniel P. Berrange wrote: > On Wed, Dec 23, 2015 at 11:14:12AM +0800, Fam Zheng wrote: > > As an alternative, can we introduce .bdrv_flock() in protocol drivers, with > > similar semantics to flock(2) or lockf(3)? That way all formats can benefit, > > and a program crash will automatically drop the lock. > > FWIW, the libvirt locking daemon (virtlockd) will already attempt to take > out locks using fcntl()/lockf() on all disk images associated with a VM. Is it even possible without QEMU cooperating? In particular in complex cases with e.g. backing chains? This was exactly the reason why we designed the "lock" option to take an argument describing the locking mechanism to be used (see the tentative patchset Denis posted in this thread). The only one currently implemented is flock()-based; however it can be extended to other mechanisms like network / cluster / SAN lock managers, etc. In particular, it can be made to talk to virtlockd. Roman.
[Qemu-devel] [PATCH] sheepdog: allow to delete snapshot
From: Vasiliy Tolstov
This patch implements a blockdriver function bdrv_snapshot_delete() in
the sheepdog driver. With the new function, snapshots of sheepdog can
be deleted from libvirt.
Cc: Jeff Cody
Signed-off-by: Hitoshi Mitake
Signed-off-by: Vasiliy Tolstov
---
block/sheepdog.c | 125 ++-
1 file changed, 123 insertions(+), 2 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index d80e4ed..0a4f2fc 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -283,6 +283,12 @@ static inline bool is_snapshot(struct SheepdogInode *inode)
return !!inode->snap_ctime;
}
+static inline size_t count_data_objs(const struct SheepdogInode *inode)
+{
+return DIV_ROUND_UP(inode->vdi_size,
+(1UL << inode->block_size_shift));
+}
+
#undef DPRINTF
#ifdef DEBUG_SDOG
#define DPRINTF(fmt, args...) \
@@ -2479,13 +2485,128 @@ out:
return ret;
}
+#define NR_BATCHED_DISCARD 128
+
+static bool remove_objects(BDRVSheepdogState *s)
+{
+int fd, i = 0, nr_objs = 0;
+Error *local_err = NULL;
+int ret = 0;
+bool result = true;
+SheepdogInode *inode = &s->inode;
+
+fd = connect_to_sdog(s, &local_err);
+if (fd < 0) {
+error_report_err(local_err);
+return false;
+}
+
+nr_objs = count_data_objs(inode);
+while (i < nr_objs) {
+int start_idx, nr_filled_idx;
+
+while (i < nr_objs && !inode->data_vdi_id[i]) {
+i++;
+}
+start_idx = i;
+
+nr_filled_idx = 0;
+while (i < nr_objs && nr_filled_idx < NR_BATCHED_DISCARD) {
+if (inode->data_vdi_id[i]) {
+inode->data_vdi_id[i] = 0;
+nr_filled_idx++;
+}
+
+i++;
+}
+
+ret = write_object(fd, s->aio_context,
+ (char *)&inode->data_vdi_id[start_idx],
+ vid_to_vdi_oid(s->inode.vdi_id), inode->nr_copies,
+ (i - start_idx) * sizeof(uint32_t),
+ offsetof(struct SheepdogInode,
+data_vdi_id[start_idx]),
+ false, s->cache_flags);
+if (ret < 0) {
+error_report("failed to discard snapshot inode.");
+result = false;
+goto out;
+}
+}
+
+out:
+closesocket(fd);
+return result;
+}
+
static int sd_snapshot_delete(BlockDriverState *bs,
const char *snapshot_id,
const char *name,
Error **errp)
{
-/* FIXME: Delete specified snapshot id. */
-return 0;
+uint32_t snap_id = 0;
+char snap_tag[SD_MAX_VDI_TAG_LEN];
+Error *local_err = NULL;
+int fd, ret;
+char buf[SD_MAX_VDI_LEN + SD_MAX_VDI_TAG_LEN];
+BDRVSheepdogState *s = bs->opaque;
+unsigned int wlen = SD_MAX_VDI_LEN + SD_MAX_VDI_TAG_LEN, rlen = 0;
+uint32_t vid;
+SheepdogVdiReq hdr = {
+.opcode = SD_OP_DEL_VDI,
+.data_length = wlen,
+.flags = SD_FLAG_CMD_WRITE,
+};
+SheepdogVdiRsp *rsp = (SheepdogVdiRsp *)&hdr;
+
+if (!remove_objects(s)) {
+return -1;
+}
+
+memset(buf, 0, sizeof(buf));
+memset(snap_tag, 0, sizeof(snap_tag));
+pstrcpy(buf, SD_MAX_VDI_LEN, s->name);
+if (qemu_strtoul(snapshot_id, NULL, 10, (unsigned long *)&snap_id)) {
+return -1;
+}
+
+if (snap_id) {
+hdr.snapid = snap_id;
+} else {
+pstrcpy(snap_tag, sizeof(snap_tag), snapshot_id);
+pstrcpy(buf + SD_MAX_VDI_LEN, SD_MAX_VDI_TAG_LEN, snap_tag);
+}
+
+ret = find_vdi_name(s, s->name, snap_id, snap_tag, &vid, true,
+&local_err);
+if (ret) {
+return ret;
+}
+
+fd = connect_to_sdog(s, &local_err);
+if (fd < 0) {
+error_report_err(local_err);
+return -1;
+}
+
+ret = do_req(fd, s->aio_context, (SheepdogReq *)&hdr,
+ buf, &wlen, &rlen);
+closesocket(fd);
+if (ret) {
+return ret;
+}
+
+switch (rsp->result) {
+case SD_RES_NO_VDI:
+error_report("%s was already deleted", s->name);
+case SD_RES_SUCCESS:
+break;
+default:
+error_report("%s, %s", sd_strerror(rsp->result), s->name);
+return -1;
+}
+
+return ret;
}
static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
--
1.9.1
Re: [Qemu-devel] [PULL 0/4] xen-2015-12-22
On 22 December 2015 at 16:20, Stefano Stabellini wrote: > The following changes since commit c3626ca7df027dabf0568284360a23faf18f0884: > > Update version for v2.5.0-rc3 release (2015-12-07 17:47:40 +) > > are available in the git repository at: > > git://xenbits.xen.org/people/sstabellini/qemu-dm.git tags/xen-2015-12-22 > > for you to fetch changes up to fc3e493bc8e96ef4bf7ae4f035f43cb39382c936: > > xen_disk: treat "vhd" as "vpc" (2015-12-11 17:02:37 +) > > > Xen 2015/12/22 > > > Jan Beulich (3): > xen/MSI-X: latch MSI-X table writes > xen/MSI-X: really enforce alignment > xen/pass-through: correctly deal with RW1C bits > > Stefano Stabellini (1): > xen_disk: treat "vhd" as "vpc" Applied, thanks. -- PMM
Re: [Qemu-devel] [Qemu-block] [PATCH 00/10] qcow2: Implement image locking
On Wed, Dec 23, 2015 at 03:15:50PM +0300, Roman Kagan wrote: > On Wed, Dec 23, 2015 at 10:47:22AM +, Daniel P. Berrange wrote: > > On Wed, Dec 23, 2015 at 11:14:12AM +0800, Fam Zheng wrote: > > > As an alternative, can we introduce .bdrv_flock() in protocol drivers, > > > with > > > similar semantics to flock(2) or lockf(3)? That way all formats can > > > benefit, > > > and a program crash will automatically drop the lock. > > > > FWIW, the libvirt locking daemon (virtlockd) will already attempt to take > > out locks using fcntl()/lockf() on all disk images associated with a VM. > > Is it even possible without QEMU cooperating? In particular in complex > cases with e.g. backing chains? Yes, libvirt already has to know & understand exactly what chains are in use in order to grant correct permissions via SELinux/AppArmour. Once it knows that it can also deal with acquiring suitable locks. > This was exactly the reason why we designed the "lock" option to take an > argument describing the locking mechanism to be used (see the tentative > patchset Denis posted in this thread). The only one currently > implemented is flock()-based; however it can be extended to other > mechanisms like network / cluster / SAN lock managers, etc. In > particular, it can be made to talk to virtlockd. NB flock() doesn't work reliably / portably on NFS. Many impls would treat it as a no-op. Other impls would only acquire the lock on the local NFS client, not the server. Apparently Linux now[1] transparently converts flock() into fcntl() locks on NFS only, so you now have the problem that any close() will release the lock. So IMHO flock() is even less usable than fcntl() as a result. Regards, Daniel [1]http://0pointer.de/blog/projects/locking.html -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [Qemu-block] [PATCH 00/10] qcow2: Implement image locking
On Wed, Dec 23, 2015 at 03:15:50PM +0300, Roman Kagan wrote: > On Wed, Dec 23, 2015 at 10:47:22AM +, Daniel P. Berrange wrote: > > On Wed, Dec 23, 2015 at 11:14:12AM +0800, Fam Zheng wrote: > > > As an alternative, can we introduce .bdrv_flock() in protocol drivers, > > > with > > > similar semantics to flock(2) or lockf(3)? That way all formats can > > > benefit, > > > and a program crash will automatically drop the lock. > > > > FWIW, the libvirt locking daemon (virtlockd) will already attempt to take > > out locks using fcntl()/lockf() on all disk images associated with a VM. > > Is it even possible without QEMU cooperating? In particular in complex > cases with e.g. backing chains? > > This was exactly the reason why we designed the "lock" option to take an > argument describing the locking mechanism to be used (see the tentative > patchset Denis posted in this thread). The only one currently > implemented is flock()-based; however it can be extended to other > mechanisms like network / cluster / SAN lock managers, etc. In > particular, it can be made to talk to virtlockd. NB, libvirt generally considers QEMU to be untrustworthy, which is another reason why we use virtlockd to acquire the locks *prior* to granting QEMU any access to the file(s). On this basis we would not really trust QEMU to do acquire/release locks itself by talking to virtlockd. Indeed, we'd not really trust QEMU locking at all, no matter what mechanism it used - we want strong guarantee of locking regardless of whether QEMU is broken / compromised. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH 2/5] block: implemented bdrv_lock_image for raw file
On Wed, Dec 23, 2015 at 10:46:53AM +0300, Denis V. Lunev wrote:
> From: Olga Krishtal
>
> To lock the image file flock (LockFileEx) is used.
> We lock file handle/descriptor. If lock is failed -
> an error is returned.
>
> In win32 realization we can lock reagion of bytes within the file.
> For this reason we at first have to get file size and only than lock it.
>
> Signed-off-by: Olga Krishtal
> Signed-off-by: Denis V. Lunev
> CC: Kevin Wolf
> CC: Max Reitz
> CC: Eric Blake
> CC: Fam Zheng
> ---
> block/raw-posix.c | 15 +++
> block/raw-win32.c | 19 +++
> 2 files changed, 34 insertions(+)
>
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 076d070..6226a5c 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -33,6 +33,7 @@
> #include "raw-aio.h"
> #include "qapi/util.h"
> #include "qapi/qmp/qstring.h"
> +#include
>
> #if defined(__APPLE__) && (__MACH__)
> #include
> @@ -576,6 +577,19 @@ fail:
> return ret;
> }
>
> +static int raw_lock_image(BlockDriverState *bs, BdrvLockImage lock)
> +{
> +int ret;
> +if (lock != BDRV_LOCK_IMAGE_LOCKFILE) {
> +return -ENOTSUP;
> +}
> +ret = flock(((BDRVRawState *)(bs->opaque))->fd, LOCK_EX|LOCK_NB);
> +if (ret != 0) {
> +return -ret;
> +}
> +return ret;
> +}
flock() is a pretty bad choice wrt to NFS. Historically it was often
a no-op. Some impls treat it as a client-local lock and not a server
side lock. Linux apparently now converts flock locks to fcntl locks,
but on NFS only. As a result they'll suffer from the problem that
a close() on any file descriptor pointing to the file will release
the lock. So IMHO both flock() and fcntl() are unusable in practice
from within QEMU, as I don't think it is practical to guarantee
QEMU won't accidentally release the lock by closing another file
descriptor pointing to the same file. If you want to use flock or
fcntl() and provide a strong safety guarantee the only option is to
acquire the locks in a dedicated process prior to giving QEMU access
to the files, which is what libvirt does with its virtlockd daemon.
Regards,
Daniel
[1] http://0pointer.de/blog/projects/locking.html
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [Qemu-block] [PATCH 00/10] qcow2: Implement image locking
On 12/23/2015 03:29 PM, Daniel P. Berrange wrote: On Wed, Dec 23, 2015 at 03:15:50PM +0300, Roman Kagan wrote: On Wed, Dec 23, 2015 at 10:47:22AM +, Daniel P. Berrange wrote: On Wed, Dec 23, 2015 at 11:14:12AM +0800, Fam Zheng wrote: As an alternative, can we introduce .bdrv_flock() in protocol drivers, with similar semantics to flock(2) or lockf(3)? That way all formats can benefit, and a program crash will automatically drop the lock. FWIW, the libvirt locking daemon (virtlockd) will already attempt to take out locks using fcntl()/lockf() on all disk images associated with a VM. Is it even possible without QEMU cooperating? In particular in complex cases with e.g. backing chains? Yes, libvirt already has to know & understand exactly what chains are in use in order to grant correct permissions via SELinux/AppArmour. Once it knows that it can also deal with acquiring suitable locks. This was exactly the reason why we designed the "lock" option to take an argument describing the locking mechanism to be used (see the tentative patchset Denis posted in this thread). The only one currently implemented is flock()-based; however it can be extended to other mechanisms like network / cluster / SAN lock managers, etc. In particular, it can be made to talk to virtlockd. NB flock() doesn't work reliably / portably on NFS. Many impls would treat it as a no-op. Other impls would only acquire the lock on the local NFS client, not the server. Apparently Linux now[1] transparently converts flock() into fcntl() locks on NFS only, so you now have the problem that any close() will release the lock. So IMHO flock() is even less usable than fcntl() as a result. Regards, Daniel [1]http://0pointer.de/blog/projects/locking.html Do you suggest to implement connection FROM qemu-img etc stuff to libvirt and query libvirt about this? This is absolutely fine actually, why not. We can made lock mechanics with type 'libvirt' exactly in the same way as flock. This approach should satisfy all needs and users. Isn't it? Den
Re: [Qemu-devel] [Qemu-block] [PATCH 00/10] qcow2: Implement image locking
On Wed, Dec 23, 2015 at 03:41:01PM +0300, Denis V. Lunev wrote: > On 12/23/2015 03:29 PM, Daniel P. Berrange wrote: > >On Wed, Dec 23, 2015 at 03:15:50PM +0300, Roman Kagan wrote: > >>On Wed, Dec 23, 2015 at 10:47:22AM +, Daniel P. Berrange wrote: > >>>On Wed, Dec 23, 2015 at 11:14:12AM +0800, Fam Zheng wrote: > As an alternative, can we introduce .bdrv_flock() in protocol drivers, > with > similar semantics to flock(2) or lockf(3)? That way all formats can > benefit, > and a program crash will automatically drop the lock. > >>>FWIW, the libvirt locking daemon (virtlockd) will already attempt to take > >>>out locks using fcntl()/lockf() on all disk images associated with a VM. > >>Is it even possible without QEMU cooperating? In particular in complex > >>cases with e.g. backing chains? > >Yes, libvirt already has to know & understand exactly what chains are > >in use in order to grant correct permissions via SELinux/AppArmour. > >Once it knows that it can also deal with acquiring suitable locks. > > > >>This was exactly the reason why we designed the "lock" option to take an > >>argument describing the locking mechanism to be used (see the tentative > >>patchset Denis posted in this thread). The only one currently > >>implemented is flock()-based; however it can be extended to other > >>mechanisms like network / cluster / SAN lock managers, etc. In > >>particular, it can be made to talk to virtlockd. > >NB flock() doesn't work reliably / portably on NFS. Many impls > >would treat it as a no-op. Other impls would only acquire the > >lock on the local NFS client, not the server. Apparently Linux > >now[1] transparently converts flock() into fcntl() locks on NFS > >only, so you now have the problem that any close() will release > >the lock. So IMHO flock() is even less usable than fcntl() as > >a result. > > > >Regards, > >Daniel > > > >[1]http://0pointer.de/blog/projects/locking.html > Do you suggest to implement connection FROM qemu-img etc stuff > to libvirt and query libvirt about this? > > This is absolutely fine actually, why not. We can made lock mechanics > with type 'libvirt' exactly in the same way as flock. This approach > should satisfy all needs and users. You want libvirt to use its locking APIs any time it has to invoke qemu-img. For case where people are not using libvirt, but running qemu-img directly having qemu-img call back into libvirt isn't going to help IMHO. Those people are already not paying attention to the docs, so they're also not going to remember to add the command line to tell qemu-img to talk to libvirt. So its pretty pointless to have qemu-img talk to libvirt IMHO, as well as adding complexity by creating a mutual two-way dependancy which is undesirable. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [Qemu-block] [PATCH 00/10] qcow2: Implement image locking
On 12/23/2015 03:34 PM, Daniel P. Berrange wrote: On Wed, Dec 23, 2015 at 03:15:50PM +0300, Roman Kagan wrote: On Wed, Dec 23, 2015 at 10:47:22AM +, Daniel P. Berrange wrote: On Wed, Dec 23, 2015 at 11:14:12AM +0800, Fam Zheng wrote: As an alternative, can we introduce .bdrv_flock() in protocol drivers, with similar semantics to flock(2) or lockf(3)? That way all formats can benefit, and a program crash will automatically drop the lock. FWIW, the libvirt locking daemon (virtlockd) will already attempt to take out locks using fcntl()/lockf() on all disk images associated with a VM. Is it even possible without QEMU cooperating? In particular in complex cases with e.g. backing chains? This was exactly the reason why we designed the "lock" option to take an argument describing the locking mechanism to be used (see the tentative patchset Denis posted in this thread). The only one currently implemented is flock()-based; however it can be extended to other mechanisms like network / cluster / SAN lock managers, etc. In particular, it can be made to talk to virtlockd. NB, libvirt generally considers QEMU to be untrustworthy, which is another reason why we use virtlockd to acquire the locks *prior* to granting QEMU any access to the file(s). On this basis we would not really trust QEMU to do acquire/release locks itself by talking to virtlockd. Indeed, we'd not really trust QEMU locking at all, no matter what mechanism it used - we want strong guarantee of locking regardless of whether QEMU is broken / compromised. Regards, Daniel this is not the case we are trying to solve here. Here customer accidentally called 'qemu-img snapshot' and face his doom in ruined image. How can we will be able to find proper libvirtd in the case of network filesystem inside client swarm? This daemon is local to the host. Filesystem locking can be used in the hope that setup is consistent. Den
Re: [Qemu-devel] [PULL 00/55] acpi, pc features
On 22 December 2015 at 16:52, Michael S. Tsirkin wrote: > The following changes since commit 5dc42c186d63b7b338594fc071cf290805dcc5a5: > > Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' > into staging (2015-12-22 14:21:42 +) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream > > for you to fetch changes up to 5530427f0ca240b972f25ef0413fb966f96dfb05: > > acpi: extend aml_and() to accept target argument (2015-12-22 18:39:21 +0200) > > > acpi, pc features > > pxb support for q35 > nvdimm support > most of ipmi support > part of DSDT rewrite > > Signed-off-by: Michael S. Tsirkin > > Applied, thanks. -- PMM
Re: [Qemu-devel] [Qemu-block] [PATCH 00/10] qcow2: Implement image locking
On Wed, Dec 23, 2015 at 03:47:00PM +0300, Denis V. Lunev wrote: > On 12/23/2015 03:34 PM, Daniel P. Berrange wrote: > >On Wed, Dec 23, 2015 at 03:15:50PM +0300, Roman Kagan wrote: > >>On Wed, Dec 23, 2015 at 10:47:22AM +, Daniel P. Berrange wrote: > >>>On Wed, Dec 23, 2015 at 11:14:12AM +0800, Fam Zheng wrote: > As an alternative, can we introduce .bdrv_flock() in protocol drivers, > with > similar semantics to flock(2) or lockf(3)? That way all formats can > benefit, > and a program crash will automatically drop the lock. > >>>FWIW, the libvirt locking daemon (virtlockd) will already attempt to take > >>>out locks using fcntl()/lockf() on all disk images associated with a VM. > >>Is it even possible without QEMU cooperating? In particular in complex > >>cases with e.g. backing chains? > >> > >>This was exactly the reason why we designed the "lock" option to take an > >>argument describing the locking mechanism to be used (see the tentative > >>patchset Denis posted in this thread). The only one currently > >>implemented is flock()-based; however it can be extended to other > >>mechanisms like network / cluster / SAN lock managers, etc. In > >>particular, it can be made to talk to virtlockd. > >NB, libvirt generally considers QEMU to be untrustworthy, which is > >another reason why we use virtlockd to acquire the locks *prior* > >to granting QEMU any access to the file(s). On this basis we would > >not really trust QEMU to do acquire/release locks itself by talking > >to virtlockd. Indeed, we'd not really trust QEMU locking at all, no > >matter what mechanism it used - we want strong guarantee of locking > >regardless of whether QEMU is broken / compromised. > > > this is not the case we are trying to solve here. Here customer accidentally > called 'qemu-img snapshot' and face his doom in ruined image. You're merely describing one out of many possible ways to ruin the image. Running multiple QEMU system emulators pointing to the same image will equally trash it. Or an admin mistakenly adding the same image to the same QEMU twice eg once as a primary image, and once mistakenly via a backing file. Or a broken / compromised QEMU mistakenly/intentionally acquiring the wrong locks or not any locks. Any locking mechanism has to consider all the possible ways of doom. > How can we will be able to find proper libvirtd in the case of network > filesystem inside client swarm? This daemon is local to the host. > Filesystem locking can be used in the hope that setup is consistent. We have one virtlockd on each host, and they would be configured to use a common lockspace on the shared filesystem, so the locks acquired on one host would be visible to the other host & vica-verca. This works reasonably reliably with fcntl(), at least more so than with flock(). Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH] Xen PCI passthrough: convert to realize()
Hi Stefano,
first of all, thanks for your quick response:)
On 12/23/2015 08:03 PM, Stefano Stabellini wrote:
On Wed, 23 Dec 2015, Cao jin wrote:
Signed-off-by: Cao jin
---
Since the callchain is pretty deep & error path is very much too, so I made the
patch based on the principal: catch/report the most necessary error msg with
smallest modification.(So you can see I don`t change some functions to void,
despite they coule be)
Thanks Cao.
For consistency with the other functions, I think it would be better to
change all functions to return void or none.
Ok, I`ll select one style may with the smallest modification;)
Also it might be nice to split the patch in a series.
Yup, and the patches should be independent from each other?
The patch as is fails to build:
qemu/hw/xen/xen_pt_config_init.c: In function ‘xen_pt_config_init’:
qemu/hw/xen/xen_pt_config_init.c:2061:42: error: ‘rc’ may be used uninitialized
in this func
really weird...last patch you remind me that it cannot compile, make me
find that my computer didn`t install xen-devel package, then I installed
it right away. But this time, it really can compile on my
computeranyway, I will check it out later.
hw/xen/xen-host-pci-device.c | 79 +++-
hw/xen/xen-host-pci-device.h | 5 +--
hw/xen/xen_pt.c | 67 +++--
hw/xen/xen_pt.h | 5 +--
hw/xen/xen_pt_config_init.c | 47 +-
hw/xen/xen_pt_graphics.c | 6 ++--
6 files changed, 118 insertions(+), 91 deletions(-)
diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
index 7d8a023..1ab6d97 100644
--- a/hw/xen/xen-host-pci-device.c
+++ b/hw/xen/xen-host-pci-device.c
@@ -43,13 +43,14 @@ static int xen_host_pci_sysfs_path(const XenHostPCIDevice
*d,
/* The output is truncated, or some other error was encountered */
return -ENODEV;
}
+
return 0;
}
I would prefer to keep stylistic changes separate, especially the ones
in functions which would be otherwise left unmodified. Maybe you could
move them to a separate patch?
I can do that.
[...]
+
if (i != PCI_NUM_REGIONS) {
/* Invalid format or input to short */
-rc = -ENODEV;
+error_setg(errp, "Invalid format or input to short");
^too short
How about printing all the string in buf? like:
"Invalid format or input to short: %s", buf
for all the other comments below: will fix them up:)
}
out:
close(fd);
-return rc;
}
[...]
--
Yours Sincerely,
Cao Jin
Re: [Qemu-devel] [PATCH v3 2/2] tests: update expected SSDT for floppy changes
On Tue, Dec 22, 2015 at 06:41:47PM +0200, Michael S. Tsirkin wrote: > On Fri, Dec 18, 2015 at 10:32:30PM +0300, Roman Kagan wrote: > > Update the expected SSDTs to reflect the changes introduced in the > > previous patch. > > > > Signed-off-by: Roman Kagan > > Signed-off-by: Denis V. Lunev > > CC: Michael S. Tsirkin > > CC: Igor Mammedov > > CC: Paolo Bonzini > > CC: Richard Henderson > > CC: Eduardo Habkost > > CC: John Snow > > CC: Kevin Wolf > > Something strange is going on here. > If I apply your patch and this one on top, I get > a diff in SSDT. Aren't you by chance applying it on top of other patches that may affect SSDT? I double-checked the series on top of commit 5dc42c186d63b7b338594fc071cf290805dcc5a5 Merge: c595b21 7236975 Author: Peter Maydell Date: Tue Dec 22 14:21:42 2015 + Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging and it passes OK... Roman.
Re: [Qemu-devel] [PATCH v3 0/5] linux-user: manage SOCK_PACKET socket type
Le 21/12/2015 16:14, Riku Voipio a écrit : > On Fri, Dec 18, 2015 at 04:13:20PM +0100, Laurent Vivier wrote: >> Le 29/10/2015 00:12, Peter Maydell a écrit : >>> On 28 October 2015 at 20:40, Laurent Vivier wrote: This is obsolete, but if we want to use dhcp with some distros (like debian ppc 8.2 jessie), we need it. bind() uses an interface name instead an interface index, and socket() needs network order value to encode the protocol. v3: update cover letter message, insert Reviewed-by: in PATCH 1 and PATCH 2 insert fd_trans_target_to_host_addr into target_to_host_sockaddr and pass fd, check fd is >= 0, rename packet_target_to_host_addr to packet_target_to_host_sockaddr v2: Split the patch in 4 parts to manage protocol endianness (socket()) and interface name (bind()) in different patches. Use TargetFdTrans array to manage the SOCK_PACKET type special case in bind(). The two others patches are here to introduce a new function in TargetFdTrans to translate sockaddr data structure (rename previous functions to be clear). Laurent Fiver (5): >>> >>> This name doesn't match the names on the actual patch mails, >>> but those are right so I guess it doesn't matter. >>> linux-user: SOCK_PACKET uses network endian to encode protocol in socket() linux-user: rename TargetFdFunc to TargetFdDataFunc, and structure fields accordingly linux-user: add a function hook to translate sockaddr linux-user: manage bind with a socket of SOCK_PACKET type. linux-user: check fd is >= 0 in fd_trans_host_to_target_data/fd_trans_host_to_target_addr >>> >>> Series >>> Reviewed-by: Peter Maydell > >> Can we have this series applied ? > > I'll create next pull req next week, right now I'm a bit busy. If someone > else wants to merge the series before me, you have my Acked-by. If you want, I can do the pull request: - I take your branch linux-user-for-upstream from https://git.linaro.org/people/riku.voipio/qemu.git - remove "linux-user: Fix array bounds in errno conversion" because it is broken, - add "linux-user: manage SOCK_PACKET socket type" I'd like to add "linux-user, sh4: fix signal retcode address", "linux-user: Enable sigaltstack syscall for sh4" but they have no reviewer except me. Laurent
[Qemu-devel] [PATCH v3] qemu-char: add logfile facility to all chardev backends
Typically a UNIX guest OS will log boot messages to a serial
port in addition to any graphical console. An admin user
may also wish to use the serial port for an interactive
console. A virtualization management system may wish to
collect system boot messages by logging the serial port,
but also wish to allow admins interactive access.
Currently providing such a feature forces the mgmt app
to either provide 2 separate serial ports, one for
logging boot messages and one for interactive console
login, or to proxy all output via a separate service
that can multiplex the two needs onto one serial port.
While both are valid approaches, they each have their
own downsides. The former causes confusion and extra
setup work for VM admins creating disk images. The latter
places an extra burden to re-implement much of the QEMU
chardev backends logic in libvirt or even higher level
mgmt apps and adds extra hops in the data transfer path.
A simpler approach that is satisfactory for many use
cases is to allow the QEMU chardev backends to have a
"logfile" property associated with them.
$QEMU -chardev socket,host=localhost,port=9000,\
server=on,nowait,id-charserial0,\
logfile=/var/log/libvirt/qemu/test-serial0.log
-device isa-serial,chardev=charserial0,id=serial0
This patch introduces a 'ChardevCommon' struct which
is setup as a base for all the ChardevBackend types.
Ideally this would be registered directly as a base
against ChardevBackend, rather than each type, but
the QAPI generator doesn't allow that since the
ChardevBackend is a non-discriminated union. The
ChardevCommon struct provides the optional 'logfile'
parameter, as well as 'logappend' which controls
whether QEMU truncates or appends (default truncate).
Signed-off-by: Daniel P. Berrange
---
Changed in v3:
- Pass ChardevCommon into qemu_chr_alloc() (Paolo)
- Add missing conversion of baum, msmouse and spice
chardev (Paolo)
- Avoid accessing ChardevBackend->u.data and instead
cast to ChardevCommon explicitly (Eric)
- Specify O_APPEND if O_TRUNC is not desired (Eric/Paolo)
I've not changed the QAPI schema, since the discussion
around Eric's points didn't come to a clear conclusion
yet. This just address the code points.
backends/baum.c | 7 +-
backends/msmouse.c| 6 +-
gdbstub.c | 3 +-
include/sysemu/char.h | 9 +-
qapi-schema.json | 49 +++---
qemu-char.c | 243 +-
qemu-options.hx | 41 +
spice-qemu-char.c | 20 +++--
ui/console.c | 6 +-
9 files changed, 302 insertions(+), 82 deletions(-)
diff --git a/backends/baum.c b/backends/baum.c
index 723c658..ba32b61 100644
--- a/backends/baum.c
+++ b/backends/baum.c
@@ -566,6 +566,7 @@ static CharDriverState *chr_baum_init(const char *id,
ChardevReturn *ret,
Error **errp)
{
+ChardevCommon *common = qapi_ChardevDummy_base(backend->u.braille);
BaumDriverState *baum;
CharDriverState *chr;
brlapi_handle_t *handle;
@@ -576,8 +577,12 @@ static CharDriverState *chr_baum_init(const char *id,
#endif
int tty;
+chr = qemu_chr_alloc(common, errp);
+if (!chr) {
+return NULL;
+}
baum = g_malloc0(sizeof(BaumDriverState));
-baum->chr = chr = qemu_chr_alloc();
+baum->chr = chr;
chr->opaque = baum;
chr->chr_write = baum_write;
diff --git a/backends/msmouse.c b/backends/msmouse.c
index 0126fa0..476dab5 100644
--- a/backends/msmouse.c
+++ b/backends/msmouse.c
@@ -68,9 +68,13 @@ static CharDriverState *qemu_chr_open_msmouse(const char *id,
ChardevReturn *ret,
Error **errp)
{
+ChardevCommon *common = qapi_ChardevDummy_base(backend->u.msmouse);
CharDriverState *chr;
-chr = qemu_chr_alloc();
+chr = qemu_chr_alloc(common, errp);
+if (!chr) {
+return NULL;
+}
chr->chr_write = msmouse_chr_write;
chr->chr_close = msmouse_chr_close;
chr->explicit_be_open = true;
diff --git a/gdbstub.c b/gdbstub.c
index 9c29aa0..ac81195 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1732,6 +1732,7 @@ int gdbserver_start(const char *device)
char gdbstub_device_name[128];
CharDriverState *chr = NULL;
CharDriverState *mon_chr;
+ChardevCommon common = { 0 };
if (!device)
return -1;
@@ -1768,7 +1769,7 @@ int gdbserver_start(const char *device)
qemu_add_vm_change_state_handler(gdb_vm_state_change, NULL);
/* Initialize a monitor terminal for gdb */
-mon_chr = qemu_chr_alloc();
+mon_chr = qemu_chr_alloc(&common, NULL);
mon_chr->chr_write = gdb_monitor_write;
monitor_init(mon_chr, 0);
} else {
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index aff193f..598dd2b 100644
--- a/include/sysemu/char.h
+
Re: [Qemu-devel] [PULL v1 0/3] Misc I/O channel fixes
On 23 December 2015 at 10:57, Daniel P. Berrange wrote: > The following changes since commit 5dc42c186d63b7b338594fc071cf290805dcc5a5: > > Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' > into staging (2015-12-22 14:21:42 +) > > are available in the git repository at: > > git://github.com/berrange/qemu tags/pull-io-fixes-2015-12-23-1 > > for you to fetch changes up to 7b3c618ad0cd0154993b5b5dbd34e0010960585a: > > io: fix stack allocation when sending of file descriptors (2015-12-23 > 10:53:03 +) > > > Merge misc I/O channel fixes > > > Daniel P. Berrange (3): > io: bind to loopback IP addrs in test suite > io: fix setting of QIO_CHANNEL_FEATURE_FD_PASS on server connections > io: fix stack allocation when sending of file descriptors > > io/channel-socket.c| 17 -- > tests/test-io-channel-socket.c | 129 > +++-- > 2 files changed, 134 insertions(+), 12 deletions(-) > -- > 2.5.0 > Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH 3/4] backends/hostmem-file: fix fb->mem_path leak
On Wed, 23 Dec 2015 15:43:20 +0800
Li Zhijian wrote:
> Signed-off-by: Li Zhijian
> ---
> backends/hostmem-file.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> index e9b6d21..5a73fd0 100644
> --- a/backends/hostmem-file.c
> +++ b/backends/hostmem-file.c
> @@ -116,11 +116,19 @@ file_backend_instance_init(Object *o)
> set_mem_path, NULL);
> }
>
> +static void file_backend_instance_finalize(Object *o)
> +{
> +HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
> +
> +g_free(fb->mem_path);
> +}
> +
> static const TypeInfo file_backend_info = {
> .name = TYPE_MEMORY_BACKEND_FILE,
> .parent = TYPE_MEMORY_BACKEND,
> .class_init = file_backend_class_init,
> .instance_init = file_backend_instance_init,
> +.instance_finalize = file_backend_instance_finalize,
> .instance_size = sizeof(HostMemoryBackendFile),
> };
>
Reviewed-by: Igor Mammedov
Re: [Qemu-devel] [PATCH] change type of pci_bridge_initfn() to void
On Wed, Dec 23, 2015 at 04:53:21PM +0800, Cao jin wrote:
> Hi mst
> friendly ping again...
This does not work since then this function can not be
used as an init callback, and this is how
dec uses it.
> On 12/17/2015 09:53 AM, Cao jin wrote:
> >Ping
> >
> >On 11/30/2015 05:19 PM, Michael S. Tsirkin wrote:
> >>On Mon, Nov 30, 2015 at 05:00:44PM +0800, Cao jin wrote:
> >>>It always return 0(success), change its type to void, and modify its
> >>>caller.
> >>>Doing this can reduce a error path of its caller, and it is also good
> >>>when
> >>>convert init() to realize()
> >>>
> >>>Signed-off-by: Cao jin
> >>
> >>Sounds good, but pls remember to ping me after 2.5 is out.
> >>
> >>>---
> >>> hw/pci-bridge/i82801b11.c | 5 +
> >>> hw/pci-bridge/ioh3420.c| 6 +-
> >>> hw/pci-bridge/pci_bridge_dev.c | 8 +++-
> >>> hw/pci-bridge/xio3130_downstream.c | 6 +-
> >>> hw/pci-bridge/xio3130_upstream.c | 6 +-
> >>> hw/pci-host/apb.c | 5 +
> >>> hw/pci/pci_bridge.c| 3 +--
> >>> include/hw/pci/pci_bridge.h| 2 +-
> >>> 8 files changed, 10 insertions(+), 31 deletions(-)
> >>>
> >>>leave DEC 21154 PCI-PCI bridge unchanged because it is better to
> >>>handle it
> >>>when convert init() to realize().
> >>>
> >>>diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c
> >>>index 7e79bc0..b21bc2c 100644
> >>>--- a/hw/pci-bridge/i82801b11.c
> >>>+++ b/hw/pci-bridge/i82801b11.c
> >>>@@ -61,10 +61,7 @@ static int i82801b11_bridge_initfn(PCIDevice *d)
> >>> {
> >>> int rc;
> >>>
> >>>-rc = pci_bridge_initfn(d, TYPE_PCI_BUS);
> >>>-if (rc < 0) {
> >>>-return rc;
> >>>-}
> >>>+pci_bridge_initfn(d, TYPE_PCI_BUS);
> >>>
> >>> rc = pci_bridge_ssvid_init(d, I82801ba_SSVID_OFFSET,
> >>> I82801ba_SSVID_SVID,
> >>>I82801ba_SSVID_SSID);
> >>>diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
> >>>index cce2fdd..eead195 100644
> >>>--- a/hw/pci-bridge/ioh3420.c
> >>>+++ b/hw/pci-bridge/ioh3420.c
> >>>@@ -97,11 +97,7 @@ static int ioh3420_initfn(PCIDevice *d)
> >>> PCIESlot *s = PCIE_SLOT(d);
> >>> int rc;
> >>>
> >>>-rc = pci_bridge_initfn(d, TYPE_PCIE_BUS);
> >>>-if (rc < 0) {
> >>>-return rc;
> >>>-}
> >>>-
> >>>+pci_bridge_initfn(d, TYPE_PCIE_BUS);
> >>> pcie_port_init_reg(d);
> >>>
> >>> rc = pci_bridge_ssvid_init(d, IOH_EP_SSVID_OFFSET,
> >>>diff --git a/hw/pci-bridge/pci_bridge_dev.c
> >>>b/hw/pci-bridge/pci_bridge_dev.c
> >>>index 26aded9..bc3e1b7 100644
> >>>--- a/hw/pci-bridge/pci_bridge_dev.c
> >>>+++ b/hw/pci-bridge/pci_bridge_dev.c
> >>>@@ -52,10 +52,8 @@ static int pci_bridge_dev_initfn(PCIDevice *dev)
> >>> PCIBridgeDev *bridge_dev = PCI_BRIDGE_DEV(dev);
> >>> int err;
> >>>
> >>>-err = pci_bridge_initfn(dev, TYPE_PCI_BUS);
> >>>-if (err) {
> >>>-goto bridge_error;
> >>>-}
> >>>+pci_bridge_initfn(dev, TYPE_PCI_BUS);
> >>>+
> >>> if (bridge_dev->flags & (1 << PCI_BRIDGE_DEV_F_SHPC_REQ)) {
> >>> dev->config[PCI_INTERRUPT_PIN] = 0x1;
> >>> memory_region_init(&bridge_dev->bar, OBJECT(dev), "shpc-bar",
> >>>@@ -94,7 +92,7 @@ slotid_error:
> >>> }
> >>> shpc_error:
> >>> pci_bridge_exitfn(dev);
> >>>-bridge_error:
> >>>+
> >>> return err;
> >>> }
> >>>
> >>>diff --git a/hw/pci-bridge/xio3130_downstream.c
> >>>b/hw/pci-bridge/xio3130_downstream.c
> >>>index 86b7970..012daf3 100644
> >>>--- a/hw/pci-bridge/xio3130_downstream.c
> >>>+++ b/hw/pci-bridge/xio3130_downstream.c
> >>>@@ -61,11 +61,7 @@ static int xio3130_downstream_initfn(PCIDevice *d)
> >>> PCIESlot *s = PCIE_SLOT(d);
> >>> int rc;
> >>>
> >>>-rc = pci_bridge_initfn(d, TYPE_PCIE_BUS);
> >>>-if (rc < 0) {
> >>>-return rc;
> >>>-}
> >>>-
> >>>+pci_bridge_initfn(d, TYPE_PCIE_BUS);
> >>> pcie_port_init_reg(d);
> >>>
> >>> rc = msi_init(d, XIO3130_MSI_OFFSET, XIO3130_MSI_NR_VECTOR,
> >>>diff --git a/hw/pci-bridge/xio3130_upstream.c
> >>>b/hw/pci-bridge/xio3130_upstream.c
> >>>index eada582..434c8fd 100644
> >>>--- a/hw/pci-bridge/xio3130_upstream.c
> >>>+++ b/hw/pci-bridge/xio3130_upstream.c
> >>>@@ -56,11 +56,7 @@ static int xio3130_upstream_initfn(PCIDevice *d)
> >>> PCIEPort *p = PCIE_PORT(d);
> >>> int rc;
> >>>
> >>>-rc = pci_bridge_initfn(d, TYPE_PCIE_BUS);
> >>>-if (rc < 0) {
> >>>-return rc;
> >>>-}
> >>>-
> >>>+pci_bridge_initfn(d, TYPE_PCIE_BUS);
> >>> pcie_port_init_reg(d);
> >>>
> >>> rc = msi_init(d, XIO3130_MSI_OFFSET, XIO3130_MSI_NR_VECTOR,
> >>>diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> >>>index 599768e..e9117b9 100644
> >>>--- a/hw/pci-host/apb.c
> >>>+++ b/hw/pci-host/apb.c
> >>>@@ -636,10 +636,7 @@ static int apb_pci_bridge_initfn(PCIDevice *dev)
> >>> {
> >>> int rc;
> >>>
> >>>-rc = pci_bridge_initfn(dev, TYPE_PCI_BUS);
> >>>-if (rc < 0) {
> >>>-
Re: [Qemu-devel] [PATCH v3 2/2] tests: update expected SSDT for floppy changes
On Wed, Dec 23, 2015 at 04:08:19PM +0300, Roman Kagan wrote:
> On Tue, Dec 22, 2015 at 06:41:47PM +0200, Michael S. Tsirkin wrote:
> > On Fri, Dec 18, 2015 at 10:32:30PM +0300, Roman Kagan wrote:
> > > Update the expected SSDTs to reflect the changes introduced in the
> > > previous patch.
> > >
> > > Signed-off-by: Roman Kagan
> > > Signed-off-by: Denis V. Lunev
> > > CC: Michael S. Tsirkin
> > > CC: Igor Mammedov
> > > CC: Paolo Bonzini
> > > CC: Richard Henderson
> > > CC: Eduardo Habkost
> > > CC: John Snow
> > > CC: Kevin Wolf
> >
> > Something strange is going on here.
> > If I apply your patch and this one on top, I get
> > a diff in SSDT.
>
> Aren't you by chance applying it on top of other patches that may affect
> SSDT? I double-checked the series on top of
>
> commit 5dc42c186d63b7b338594fc071cf290805dcc5a5
> Merge: c595b21 7236975
> Author: Peter Maydell
> Date: Tue Dec 22 14:21:42 2015 +
>
> Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request'
> into staging
>
>
> and it passes OK...
>
> Roman.
This is the actual vs expected diff with both patches applied.
--- /tmp/asl-MN299X.dsl 2015-12-23 15:41:29.369837861 +0200
+++ /tmp/asl-T0S99X.dsl 2015-12-23 15:41:29.373837813 +0200
@@ -5,20 +5,20 @@
*
* Disassembling to symbolic ASL+ operators
*
- * Disassembly of /tmp/aml-VVVAAY, Wed Dec 23 15:41:29 2015
+ * Disassembly of tests/acpi-test-data/pc/SSDT, Wed Dec 23 15:41:29 2015
*
* Original Table Header:
* Signature"SSDT"
- * Length 0x09B6 (2486)
+ * Length 0x0A1C (2588)
* Revision 0x01
- * Checksum 0xD7
+ * Checksum 0x15
* OEM ID "BOCHS "
* OEM Table ID "BXPCSSDT"
* OEM Revision 0x0001 (1)
* Compiler ID "BXPC"
* Compiler Version 0x0001 (1)
*/
-DefinitionBlock ("/tmp/aml-VVVAAY.aml", "SSDT", 1, "BOCHS ", "BXPCSSDT",
0x0001)
+DefinitionBlock ("tests/acpi-test-data/pc/SSDT.aml", "SSDT", 1, "BOCHS ",
"BXPCSSDT", 0x0001)
{
External (_SB_.CPEJ, MethodObj)// 2 Arguments
@@ -26,6 +26,7 @@ DefinitionBlock ("/tmp/aml-VVVAAY.aml",
External (_SB_.CPST, MethodObj)// 1 Arguments
External (_SB_.PCI0, DeviceObj)
External (_SB_.PCI0.BNUM, FieldUnitObj)
+External (_SB_.PCI0.ISA_.FDC0, DeviceObj)
External (_SB_.PCI0.MHPD, DeviceObj)
External (_SB_.PCI0.PCEJ, MethodObj)// 2 Arguments
External (_SB_.PCI0.PCID, FieldUnitObj)
@@ -135,6 +136,40 @@ DefinitionBlock ("/tmp/aml-VVVAAY.aml",
})
}
+Scope (\_SB.PCI0.ISA.FDC0)
+{
+Device (FLPA)
+{
+Name (_ADR, Zero) // _ADR: Address
+Name (_FDI, Package (0x10) // _FDI: Floppy Drive Information
+{
+Zero,
+0x04,
+0x4F,
+0x12,
+One,
+0xAF,
+0x02,
+0x25,
+0x02,
+0x12,
+0x1B,
+0xFF,
+0x6C,
+0xF6,
+0x0F,
+0x08
+})
+}
+
+Name (_FDE, Buffer (0x14) // _FDE: Floppy Disk Enumerate
+{
+/* */ 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*
*/
+/* 0008 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*
*/
+/* 0010 */ 0x02, 0x00, 0x00, 0x00 /*
*/
+})
+}
+
Scope (\_SB)
{
Device (PCI0.PRES)
--- /tmp/asl-NKR59X.dsl 2015-12-23 15:41:30.429825430 +0200
+++ /tmp/asl-3MT59X.dsl 2015-12-23 15:41:30.432825395 +0200
@@ -5,26 +5,27 @@
*
* Disassembling to symbolic ASL+ operators
*
- * Disassembly of /tmp/aml-H6V69X, Wed Dec 23 15:41:30 2015
+ * Disassembly of tests/acpi-test-data/q35/SSDT, Wed Dec 23 15:41:30 2015
*
* Original Table Header:
* Signature"SSDT"
- * Length 0x02B3 (691)
+ * Length 0x0368 (872)
* Revision 0x01
- * Checksum 0x7D
+ * Checksum 0xA6
* OEM ID "BOCHS "
* OEM Table ID "BXPCSSDT"
* OEM Revision 0x0001 (1)
* Compiler ID "BXPC"
* Compiler Version 0x0001 (1)
*/
-DefinitionBlock ("/tmp/aml-H6V69X.aml", "SSDT", 1, "BOCHS ", "BXPCSSDT",
0x0001)
+DefinitionBlock ("tests/acpi-test-data/q35/SSDT.aml", "SSDT", 1, "BOCHS ",
"BXPCSSDT", 0x0001)
{
External (_SB_.CPEJ, MethodObj)// 2 Arguments
External (_SB_.CPMA, MethodObj)// 1 Arguments
External (_SB_.CPST, MethodObj)// 1 Arguments
External (_SB_.PCI0, DeviceObj)
+External (_SB_.PCI0.ISA_.FDC0, DeviceObj)
External (_SB_.PCI0.MHPD, DeviceObj)
Scope (\_SB.PCI0)
@@ -115,6 +116,64 @@ DefinitionBlock ("/tmp/aml-H6V69X.aml",
})
}
+Sco
Re: [Qemu-devel] [PATCH v2 0/3] virtio: cross-endian helpers fixes
On Thu, Dec 17, 2015 at 09:52:46AM +0100, Greg Kurz wrote:
> This series tries to rework cross-endian helpers for better clarity.
> It does not change behaviour, except perhaps patch 3/3 even if I could not
> measure any performance gain.
Breaks build:
CCmips64-softmmu/hw/mips/mips_malta.o
/home/mst/scm/qemu/hw/net/vhost_net.c: In function
‘vhost_net_set_vnet_endian’:
/home/mst/scm/qemu/hw/net/vhost_net.c:208:10: error: implicit
declaration of function ‘virtio_legacy_is_cross_endian’
[-Werror=implicit-function-declaration]
(virtio_legacy_is_cross_endian(dev) &&
!virtio_is_big_endian(dev))) {
^
/home/mst/scm/qemu/hw/net/vhost_net.c:208:9: error: nested extern
declaration of ‘virtio_legacy_is_cross_endian’ [-Werror=nested-externs]
(virtio_legacy_is_cross_endian(dev) &&
!virtio_is_big_endian(dev))) {
^
cc1: all warnings being treated as errors
/home/mst/scm/qemu/rules.mak:57: recipe for target 'hw/net/vhost_net.o'
failed
make[1]: *** [hw/net/vhost_net.o] Error 1
Makefile:186: recipe for target 'subdir-i386-softmmu' failed
make: *** [subdir-i386-softmmu] Error 2
please always build all architectures.
> ---
>
> Greg Kurz (3):
> virtio: move cross-endian helper to vhost
> vhost: move virtio 1.0 check to cross-endian helper
> virtio: optimize virtio_access_is_big_endian() for little-endian targets
>
>
> hw/virtio/vhost.c | 22 ++
> include/hw/virtio/virtio-access.h | 16 +++-
> 2 files changed, 21 insertions(+), 17 deletions(-)
Re: [Qemu-devel] [PULL] 9p fix
On 23 December 2015 at 11:04, Greg Kurz wrote: > The following changes since commit 5dc42c186d63b7b338594fc071cf290805dcc5a5: > > Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' > into staging (2015-12-22 14:21:42 +) > > are available in the git repository at: > > https://github.com/gkurz/qemu.git tags/for-upstream > > for you to fetch changes up to 4b3a4f2d458ca5a7c6c16ac36a8d9ac22cc253d6: > > virtio-9p: use accessor to get thread_pool (2015-12-23 10:56:58 +0100) > > > Fix a 2.5 regression. > > > Greg Kurz (1): > virtio-9p: use accessor to get thread_pool > Applied, thanks. -- PMM
[Qemu-devel] [PATCH v1 1/2] kvm/x86: Hyper-V SynIC tracepoints
Trace the following Hyper SynIC events:
* set msr
* set sint irq
* ack sint
* sint irq eoi
Signed-off-by: Andrey Smetanin
CC: Gleb Natapov
CC: Paolo Bonzini
CC: Roman Kagan
CC: Denis V. Lunev
CC: qemu-devel@nongnu.org
---
arch/x86/kvm/hyperv.c | 10 +++---
arch/x86/kvm/trace.h | 93 +++
2 files changed, 98 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 7857329..e69a823 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -152,7 +152,7 @@ static void kvm_hv_notify_acked_sint(struct kvm_vcpu *vcpu,
u32 sint)
struct kvm_vcpu_hv_stimer *stimer;
int gsi, idx, stimers_pending;
- vcpu_debug(vcpu, "Hyper-V SynIC acked sint %d\n", sint);
+ trace_kvm_hv_notify_acked_sint(vcpu->vcpu_id, sint);
if (synic->msg_page & HV_SYNIC_SIMP_ENABLE)
synic_clear_sint_msg_pending(synic, sint);
@@ -202,8 +202,8 @@ static int synic_set_msr(struct kvm_vcpu_hv_synic *synic,
if (!synic->active)
return 1;
- vcpu_debug(vcpu, "Hyper-V SynIC set msr 0x%x 0x%llx host %d\n",
- msr, data, host);
+ trace_kvm_hv_synic_set_msr(vcpu->vcpu_id, msr, data, host);
+
ret = 0;
switch (msr) {
case HV_X64_MSR_SCONTROL:
@@ -312,7 +312,7 @@ int synic_set_irq(struct kvm_vcpu_hv_synic *synic, u32 sint)
irq.level = 1;
ret = kvm_irq_delivery_to_apic(vcpu->kvm, NULL, &irq, NULL);
- vcpu_debug(vcpu, "Hyper-V SynIC set irq ret %d\n", ret);
+ trace_kvm_hv_synic_set_irq(vcpu->vcpu_id, sint, irq.vector, ret);
return ret;
}
@@ -332,7 +332,7 @@ void kvm_hv_synic_send_eoi(struct kvm_vcpu *vcpu, int
vector)
struct kvm_vcpu_hv_synic *synic = vcpu_to_synic(vcpu);
int i;
- vcpu_debug(vcpu, "Hyper-V SynIC send eoi vec %d\n", vector);
+ trace_kvm_hv_synic_send_eoi(vcpu->vcpu_id, vector);
for (i = 0; i < ARRAY_SIZE(synic->sint); i++)
if (synic_get_sint_vector(synic_read_sint(synic, i)) == vector)
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index 1203025..5be9c13 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -1025,6 +1025,99 @@ TRACE_EVENT(kvm_pi_irte_update,
__entry->pi_desc_addr)
);
+/*
+ * Tracepoint for kvm_hv_notify_acked_sint.
+ */
+TRACE_EVENT(kvm_hv_notify_acked_sint,
+ TP_PROTO(int vcpu_id, u32 sint),
+ TP_ARGS(vcpu_id, sint),
+
+ TP_STRUCT__entry(
+ __field(int, vcpu_id)
+ __field(u32, sint)
+ ),
+
+ TP_fast_assign(
+ __entry->vcpu_id = vcpu_id;
+ __entry->sint = sint;
+ ),
+
+ TP_printk("vcpu_id %d sint %u", __entry->vcpu_id, __entry->sint)
+);
+
+/*
+ * Tracepoint for synic_set_irq.
+ */
+TRACE_EVENT(kvm_hv_synic_set_irq,
+ TP_PROTO(int vcpu_id, u32 sint, int vector, int ret),
+ TP_ARGS(vcpu_id, sint, vector, ret),
+
+ TP_STRUCT__entry(
+ __field(int, vcpu_id)
+ __field(u32, sint)
+ __field(int, vector)
+ __field(int, ret)
+ ),
+
+ TP_fast_assign(
+ __entry->vcpu_id = vcpu_id;
+ __entry->sint = sint;
+ __entry->vector = vector;
+ __entry->ret = ret;
+ ),
+
+ TP_printk("vcpu_id %d sint %u vector %d ret %d",
+ __entry->vcpu_id, __entry->sint, __entry->vector,
+ __entry->ret)
+);
+
+/*
+ * Tracepoint for kvm_hv_synic_send_eoi.
+ */
+TRACE_EVENT(kvm_hv_synic_send_eoi,
+ TP_PROTO(int vcpu_id, int vector),
+ TP_ARGS(vcpu_id, vector),
+
+ TP_STRUCT__entry(
+ __field(int, vcpu_id)
+ __field(u32, sint)
+ __field(int, vector)
+ __field(int, ret)
+ ),
+
+ TP_fast_assign(
+ __entry->vcpu_id = vcpu_id;
+ __entry->vector = vector;
+ ),
+
+ TP_printk("vcpu_id %d vector %d", __entry->vcpu_id, __entry->vector)
+);
+
+/*
+ * Tracepoint for synic_set_msr.
+ */
+TRACE_EVENT(kvm_hv_synic_set_msr,
+ TP_PROTO(int vcpu_id, u32 msr, u64 data, bool host),
+ TP_ARGS(vcpu_id, msr, data, host),
+
+ TP_STRUCT__entry(
+ __field(int, vcpu_id)
+ __field(u32, msr)
+ __field(u64, data)
+ __field(bool, host)
+ ),
+
+ TP_fast_assign(
+ __entry->vcpu_id = vcpu_id;
+ __entry->msr = msr;
+ __entry->data = data;
+ __entry->host = host
+ ),
+
+ TP_printk("vcpu_id %d msr 0x%x data 0x%llx host %d",
+ __entry->vcpu_id, __entry->msr, __entry->data, __entry->host)
+);
+
#endif /* _TRACE_KVM_H */
#undef TRACE_INCLUDE_PATH
--
2.4.3
[Qemu-devel] [PATCH v1 0/2] KVM: Hyper-V SynIC tracepoints
The patches adds tracepoints inside Hyper-V SynIC and SynIC timers code. The series applies on top of 'kvm/x86: Update SynIC timers on guest entry only' previously sent. Signed-off-by: Andrey Smetanin CC: Gleb Natapov CC: Paolo Bonzini CC: Roman Kagan CC: Denis V. Lunev CC: qemu-devel@nongnu.org Andrey Smetanin (2): kvm/x86: Hyper-V SynIC tracepoints kvm/x86: Hyper-V SynIC timers tracepoints arch/x86/kvm/hyperv.c | 37 +-- arch/x86/kvm/trace.h | 263 ++ 2 files changed, 294 insertions(+), 6 deletions(-) -- 2.4.3
[Qemu-devel] [PATCH v1 2/2] kvm/x86: Hyper-V SynIC timers tracepoints
Trace the following Hyper SynIC timers events:
* periodic timer start
* one-shot timer start
* timer callback
* timer expiration and message delivery result
* timer config setup
* timer count setup
* timer cleanup
Signed-off-by: Andrey Smetanin
CC: Gleb Natapov
CC: Paolo Bonzini
CC: Roman Kagan
CC: Denis V. Lunev
CC: qemu-devel@nongnu.org
---
arch/x86/kvm/hyperv.c | 27 +++-
arch/x86/kvm/trace.h | 170 ++
2 files changed, 196 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index e69a823..d50675a 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -405,6 +405,9 @@ static void stimer_cleanup(struct kvm_vcpu_hv_stimer
*stimer)
{
struct kvm_vcpu *vcpu = stimer_to_vcpu(stimer);
+ trace_kvm_hv_stimer_cleanup(stimer_to_vcpu(stimer)->vcpu_id,
+ stimer->index);
+
hrtimer_cancel(&stimer->timer);
clear_bit(stimer->index,
vcpu_to_hv_vcpu(vcpu)->stimer_pending_bitmap);
@@ -417,6 +420,8 @@ static enum hrtimer_restart stimer_timer_callback(struct
hrtimer *timer)
struct kvm_vcpu_hv_stimer *stimer;
stimer = container_of(timer, struct kvm_vcpu_hv_stimer, timer);
+ trace_kvm_hv_stimer_callback(stimer_to_vcpu(stimer)->vcpu_id,
+stimer->index);
stimer_mark_pending(stimer, true);
return HRTIMER_NORESTART;
@@ -446,6 +451,11 @@ static int stimer_start(struct kvm_vcpu_hv_stimer *stimer)
} else
stimer->exp_time = time_now + stimer->count;
+ trace_kvm_hv_stimer_start_periodic(
+ stimer_to_vcpu(stimer)->vcpu_id,
+ stimer->index,
+ time_now, stimer->exp_time);
+
hrtimer_start(&stimer->timer,
ktime_add_ns(ktime_now,
100 * (stimer->exp_time - time_now)),
@@ -464,6 +474,10 @@ static int stimer_start(struct kvm_vcpu_hv_stimer *stimer)
return 0;
}
+ trace_kvm_hv_stimer_start_one_shot(stimer_to_vcpu(stimer)->vcpu_id,
+ stimer->index,
+ time_now, stimer->count);
+
hrtimer_start(&stimer->timer,
ktime_add_ns(ktime_now, 100 * (stimer->count - time_now)),
HRTIMER_MODE_ABS);
@@ -473,6 +487,9 @@ static int stimer_start(struct kvm_vcpu_hv_stimer *stimer)
static int stimer_set_config(struct kvm_vcpu_hv_stimer *stimer, u64 config,
bool host)
{
+ trace_kvm_hv_stimer_set_config(stimer_to_vcpu(stimer)->vcpu_id,
+ stimer->index, config, host);
+
stimer_cleanup(stimer);
if ((stimer->config & HV_STIMER_ENABLE) && HV_STIMER_SINT(config) == 0)
config &= ~HV_STIMER_ENABLE;
@@ -484,6 +501,9 @@ static int stimer_set_config(struct kvm_vcpu_hv_stimer
*stimer, u64 config,
static int stimer_set_count(struct kvm_vcpu_hv_stimer *stimer, u64 count,
bool host)
{
+ trace_kvm_hv_stimer_set_count(stimer_to_vcpu(stimer)->vcpu_id,
+ stimer->index, count, host);
+
stimer_cleanup(stimer);
stimer->count = count;
if (stimer->count == 0)
@@ -562,8 +582,13 @@ static int stimer_send_msg(struct kvm_vcpu_hv_stimer
*stimer)
static void stimer_expiration(struct kvm_vcpu_hv_stimer *stimer)
{
+ int r;
+
stimer->msg_pending = true;
- if (!stimer_send_msg(stimer)) {
+ r = stimer_send_msg(stimer);
+ trace_kvm_hv_stimer_expiration(stimer_to_vcpu(stimer)->vcpu_id,
+ stimer->index, r);
+ if (!r) {
stimer->msg_pending = false;
if (!(stimer->config & HV_STIMER_PERIODIC))
stimer->config |= ~HV_STIMER_ENABLE;
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index 5be9c13..41010d8 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -1118,6 +1118,176 @@ TRACE_EVENT(kvm_hv_synic_set_msr,
__entry->vcpu_id, __entry->msr, __entry->data, __entry->host)
);
+/*
+ * Tracepoint for stimer_set_config.
+ */
+TRACE_EVENT(kvm_hv_stimer_set_config,
+ TP_PROTO(int vcpu_id, int timer_index, u64 config, bool host),
+ TP_ARGS(vcpu_id, timer_index, config, host),
+
+ TP_STRUCT__entry(
+ __field(int, vcpu_id)
+ __field(int, timer_index)
+ __field(u64, config)
+ __field(bool, host)
+ ),
+
+ TP_fast_assign(
+ __entry->vcpu_id = vcpu_id;
+ __entry->timer_index = timer_index;
+ __entry->config = config;
+ __entry->hos
[Qemu-devel] [PATCH] qemu-char: delete send_all/recv_all helper methods
The qemu-char.c contains two helper methods send_all
and recv_all. These are in fact declared in sockets.h
so ought to have been in util/qemu-sockets.c. For added
fun the impl of recv_all is completely missing on Win32.
Fortunately there is only a single caller of these
methods, the TPM passthrough code, which is only
ever compiled on Linux. With only a single caller
these helpers are not compelling enough to keep so
inline them in the TPM code, avoiding the need to
fix the missing recv_all on Win32.
Signed-off-by: Daniel P. Berrange
---
hw/tpm/tpm_passthrough.c | 29 ++--
include/qemu/sockets.h | 2 --
qemu-char.c | 71
3 files changed, 27 insertions(+), 75 deletions(-)
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index be160c1..ab526db 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -83,12 +83,37 @@ static void tpm_passthrough_cancel_cmd(TPMBackend *tb);
static int tpm_passthrough_unix_write(int fd, const uint8_t *buf, uint32_t len)
{
-return send_all(fd, buf, len);
+int ret, remain;
+
+remain = len;
+while (len > 0) {
+ret = write(fd, buf, remain);
+if (ret < 0) {
+if (errno != EINTR && errno != EAGAIN) {
+return -1;
+}
+} else if (ret == 0) {
+break;
+} else {
+buf += ret;
+remain -= ret;
+}
+}
+return len - remain;
}
static int tpm_passthrough_unix_read(int fd, uint8_t *buf, uint32_t len)
{
-return recv_all(fd, buf, len, true);
+int ret;
+ reread:
+ret = read(fd, buf, len);
+if (ret < 0) {
+if (errno != EINTR && errno != EAGAIN) {
+return -1;
+}
+goto reread;
+}
+return ret;
}
static uint32_t tpm_passthrough_get_size_from_buffer(const uint8_t *buf)
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 74c692d..0ff3a72 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -40,8 +40,6 @@ int socket_set_nodelay(int fd);
void qemu_set_block(int fd);
void qemu_set_nonblock(int fd);
int socket_set_fast_reuse(int fd);
-int send_all(int fd, const void *buf, int len1);
-int recv_all(int fd, void *buf, int len1, bool single_read);
#ifdef WIN32
/* Windows has different names for the same constants with the same values */
diff --git a/qemu-char.c b/qemu-char.c
index 66703e3..720235d 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -695,77 +695,6 @@ static CharDriverState *qemu_chr_open_mux(const char *id,
}
-#ifdef _WIN32
-int send_all(int fd, const void *buf, int len1)
-{
-int ret, len;
-
-len = len1;
-while (len > 0) {
-ret = send(fd, buf, len, 0);
-if (ret < 0) {
-errno = WSAGetLastError();
-if (errno != WSAEWOULDBLOCK) {
-return -1;
-}
-} else if (ret == 0) {
-break;
-} else {
-buf += ret;
-len -= ret;
-}
-}
-return len1 - len;
-}
-
-#else
-
-int send_all(int fd, const void *_buf, int len1)
-{
-int ret, len;
-const uint8_t *buf = _buf;
-
-len = len1;
-while (len > 0) {
-ret = write(fd, buf, len);
-if (ret < 0) {
-if (errno != EINTR && errno != EAGAIN)
-return -1;
-} else if (ret == 0) {
-break;
-} else {
-buf += ret;
-len -= ret;
-}
-}
-return len1 - len;
-}
-
-int recv_all(int fd, void *_buf, int len1, bool single_read)
-{
-int ret, len;
-uint8_t *buf = _buf;
-
-len = len1;
-while ((len > 0) && (ret = read(fd, buf, len)) != 0) {
-if (ret < 0) {
-if (errno != EINTR && errno != EAGAIN) {
-return -1;
-}
-continue;
-} else {
-if (single_read) {
-return ret;
-}
-buf += ret;
-len -= ret;
-}
-}
-return len1 - len;
-}
-
-#endif /* !_WIN32 */
-
typedef struct IOWatchPoll
{
GSource parent;
--
2.5.0
Re: [Qemu-devel] [PATCH] Xen PCI passthrough: convert to realize()
On Wed, 23 Dec 2015, Cao jin wrote:
> Hi Stefano,
> first of all, thanks for your quick response:)
Thank you for the patch
> On 12/23/2015 08:03 PM, Stefano Stabellini wrote:
> > On Wed, 23 Dec 2015, Cao jin wrote:
> > > Signed-off-by: Cao jin
> > > ---
> > >
> > > Since the callchain is pretty deep & error path is very much too, so I
> > > made the
> > > patch based on the principal: catch/report the most necessary error msg
> > > with
> > > smallest modification.(So you can see I don`t change some functions to
> > > void,
> > > despite they coule be)
> >
> > Thanks Cao.
> >
> > For consistency with the other functions, I think it would be better to
> > change all functions to return void or none.
> >
>
> Ok, I`ll select one style may with the smallest modification;)
Fine by me
> > Also it might be nice to split the patch in a series.
> >
>
> Yup, and the patches should be independent from each other?
That would be best: each patch has to compile independently.
> > The patch as is fails to build:
> >
> > qemu/hw/xen/xen_pt_config_init.c: In function ‘xen_pt_config_init’:
> > qemu/hw/xen/xen_pt_config_init.c:2061:42: error: ‘rc’ may be used
> > uninitialized in this func
> >
>
> really weird...last patch you remind me that it cannot compile, make me find
> that my computer didn`t install xen-devel package, then I installed it right
> away. But this time, it really can compile on my computeranyway, I will
> check it out later.
I bet you don't have Xen PCI passthrough enabled. Do you have
CONFIG_XEN_PCI_PASSTHROUGH=y in i386-softmmu/config-target.mak?
> >
> > > hw/xen/xen-host-pci-device.c | 79
> > > +++-
> > > hw/xen/xen-host-pci-device.h | 5 +--
> > > hw/xen/xen_pt.c | 67 +++--
> > > hw/xen/xen_pt.h | 5 +--
> > > hw/xen/xen_pt_config_init.c | 47 +-
> > > hw/xen/xen_pt_graphics.c | 6 ++--
> > > 6 files changed, 118 insertions(+), 91 deletions(-)
> > >
> > > diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
> > > index 7d8a023..1ab6d97 100644
> > > --- a/hw/xen/xen-host-pci-device.c
> > > +++ b/hw/xen/xen-host-pci-device.c
> > > @@ -43,13 +43,14 @@ static int xen_host_pci_sysfs_path(const
> > > XenHostPCIDevice *d,
> > > /* The output is truncated, or some other error was encountered
> > > */
> > > return -ENODEV;
> > > }
> > > +
> > > return 0;
> > > }
> >
> > I would prefer to keep stylistic changes separate, especially the ones
> > in functions which would be otherwise left unmodified. Maybe you could
> > move them to a separate patch?
> >
>
> I can do that.
>
> >
> [...]
> > > +
> > > if (i != PCI_NUM_REGIONS) {
> > > /* Invalid format or input to short */
> > > -rc = -ENODEV;
> > > +error_setg(errp, "Invalid format or input to short");
> >
> > ^too short
>
> How about printing all the string in buf? like:
> "Invalid format or input to short: %s", buf
Sound good
> for all the other comments below: will fix them up:)
Thanks!
> > > }
> > >
> > > out:
> > > close(fd);
> > > -return rc;
> > > }
> > >
> [...]
> >
>
> --
> Yours Sincerely,
>
> Cao Jin
>
>
Re: [Qemu-devel] [PATCH 00/10] qcow2: Implement image locking
2015-12-22 19:46 GMT+03:00 Kevin Wolf : > Enough innocent images have died because users called 'qemu-img snapshot' > while > the VM was still running. Educating the users doesn't seem to be a working > strategy, so this series adds locking to qcow2 that refuses to access the > image > read-write from two processes. > > Eric, this will require a libvirt update to deal with qemu crashes which leave > locked images behind. The simplest thinkable way would be to unconditionally > override the lock in libvirt whenever the option is present. In that case, > libvirt VMs would be protected against concurrent non-libvirt accesses, but > not > the other way round. If you want more than that, libvirt would have to check > somehow if it was its own VM that used the image and left the lock behind. I > imagine that can't be too hard either. This breaks ability to create disk only snapshot while vm is running. Or i miss something? -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru
Re: [Qemu-devel] [PATCH v3 2/2] tests: update expected SSDT for floppy changes
On Wed, Dec 23, 2015 at 03:45:29PM +0200, Michael S. Tsirkin wrote:
> This is the actual vs expected diff with both patches applied.
Interesting... The diff suggests that qemu running in your test
environment has no floppy drives, while in mine ...
> +Scope (\_SB.PCI0.ISA.FDC0)
> +{
> +Device (FLPA)
> +{
> +Name (_ADR, Zero) // _ADR: Address
> +Name (_FDI, Package (0x10) // _FDI: Floppy Drive Information
> +{
> +Zero,
> +0x04,
> +0x4F,
> +0x12,
> +One,
[...]
> +})
> +}
> +
> +Name (_FDE, Buffer (0x14) // _FDE: Floppy Disk Enumerate
> +{
> +/* */ 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*
> */
> +/* 0008 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*
> */
> +/* 0010 */ 0x02, 0x00, 0x00, 0x00 /*
> */
> +})
> +}
... it has one 1.44M drive with correct geometry for pc machine type,
and ...
> +Scope (\_SB.PCI0.ISA.FDC0)
> +{
> +Device (FLPA)
> +{
> +Name (_ADR, Zero) // _ADR: Address
> +Name (_FDI, Package (0x10) // _FDI: Floppy Drive Information
> +{
> +Zero,
> +0x04,
> +0x,
> +Zero,
> +0x,
[...]
> +})
> +}
> +
> +Device (FLPB)
> +{
> +Name (_ADR, One) // _ADR: Address
> +Name (_FDI, Package (0x10) // _FDI: Floppy Drive Information
> +{
> +One,
> +0x04,
> +0x,
> +Zero,
> +0x,
[...]
> +})
> +}
> +
> +Name (_FDE, Buffer (0x14) // _FDE: Floppy Disk Enumerate
> +{
> +/* */ 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, /*
> */
> +/* 0008 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /*
> */
> +/* 0010 */ 0x02, 0x00, 0x00, 0x00 /*
> */
> +})
> +}
... two 1.44M drives with bogus geometry for q35.
Looking into this, thanks for the diff.
Roman.
Re: [Qemu-devel] [Qemu-block] [PATCH 00/10] qcow2: Implement image locking
On 12/23/2015 05:57 PM, Vasiliy Tolstov wrote: 2015-12-22 19:46 GMT+03:00 Kevin Wolf : Enough innocent images have died because users called 'qemu-img snapshot' while the VM was still running. Educating the users doesn't seem to be a working strategy, so this series adds locking to qcow2 that refuses to access the image read-write from two processes. Eric, this will require a libvirt update to deal with qemu crashes which leave locked images behind. The simplest thinkable way would be to unconditionally override the lock in libvirt whenever the option is present. In that case, libvirt VMs would be protected against concurrent non-libvirt accesses, but not the other way round. If you want more than that, libvirt would have to check somehow if it was its own VM that used the image and left the lock behind. I imagine that can't be too hard either. This breaks ability to create disk only snapshot while vm is running. Or i miss something? you should do this by asking running QEMU not by qemu-img, which is badly wrong. Den
Re: [Qemu-devel] [Qemu-block] [PATCH 00/10] qcow2: Implement image locking
On 12/23/2015 05:57 PM, Vasiliy Tolstov wrote: 2015-12-22 19:46 GMT+03:00 Kevin Wolf : Enough innocent images have died because users called 'qemu-img snapshot' while the VM was still running. Educating the users doesn't seem to be a working strategy, so this series adds locking to qcow2 that refuses to access the image read-write from two processes. Eric, this will require a libvirt update to deal with qemu crashes which leave locked images behind. The simplest thinkable way would be to unconditionally override the lock in libvirt whenever the option is present. In that case, libvirt VMs would be protected against concurrent non-libvirt accesses, but not the other way round. If you want more than that, libvirt would have to check somehow if it was its own VM that used the image and left the lock behind. I imagine that can't be too hard either. This breaks ability to create disk only snapshot while vm is running. Or i miss something? you should do this by asking running QEMU but not by qemu-img, which is badly wrong. Den
Re: [Qemu-devel] [Qemu-block] [PATCH 00/10] qcow2: Implement image locking
2015-12-23 18:08 GMT+03:00 Denis V. Lunev : > you should do this by asking running QEMU not by > qemu-img, which is badly wrong. > > Den Ok, if this is possible via qmp/hmp qemu, no problem. -- Vasiliy Tolstov, e-mail: v.tols...@selfip.ru
Re: [Qemu-devel] [PATCH v2] qemu-char: add logfile facility to all chardev backends
On 12/23/2015 04:24 AM, Daniel P. Berrange wrote:
> On Tue, Dec 22, 2015 at 11:45:45AM -0700, Eric Blake wrote:
>> On 12/22/2015 11:17 AM, Daniel P. Berrange wrote:
>>> Typically a UNIX guest OS will log boot messages to a serial
>>> port in addition to any graphical console. An admin user
>>> may also wish to use the serial port for an interactive
>>> console. A virtualization management system may wish to
>>> collect system boot messages by logging the serial port,
>>> but also wish to allow admins interactive access.
>>
>> [meta-review of JUST qapi decisions; code review in a separate message]
>>
>>
>> With your addition, ChardevFile now inherits from ChardevCommon, so we gain:
>>
>> { "execute": "chardev-add", "arguments": {
>> "id": "foo", "backend": { "type": "file",
>> "data": { "logfile": "logname", "out": "filename" } } } }
>
> Ok good that matches what I intended - namely that 'logfile'
> should appear at the same dict as the rest of the backend
> fields, to mirror the the fact that the C struct had all
> the common fields in the same struct too.
Or in C terms, your proposal is:
struct ChardevCommon {
char *logname; ...
};
struct ChardevFile {
/* inherited fields from ChardevCommon */
char *logname; ...
/* own fields */
char *out; ...
};
struct ChardevBackend {
ChardevBackendKind type;
union {
ChardevFile *file; ...
} u;
};
Each branch of ChardevBackend.u then has an upcast function
qapi_ChardevFile_base() that gets you to a ChardevCommon pointer.
>
>> Re-writing the existing ChardevBackend to a semantically-identical flat
>> union would be (using my shorthand syntax for anonymous base [1] and
>> anonymous branch wrappers [2]):
>>
>>
>> Your proposal, then, is that the new 'logging' fields in your
>> ChardevCommon should be made part of the base type of the
>> 'ChardevBackend' union; which would be spelled as:
>>
>> { 'enum': 'ChardevType', 'data': [ 'file', 'serial' ] }
>> { 'struct': 'ChardevCommon', 'data': {
>> 'type': 'ChardevType', '*logfile': 'str', '*logappend': bool } }
>> { 'union': 'ChardevBackend',
>> 'base': 'ChardevCommon', 'discriminator': 'type',
>> 'data': { 'file': { 'data': 'ChardevFile' },
>> 'serial': { 'data': 'ChardevHostdev' } } }
In C terms, this one would be:
struct ChardevCommon {
char *logname; ...
};
struct ChardevFile {
char *out; ...
};
struct ChardevBackend {
/* inherited fields from ChardevCommon */
char *logname; ...
/* own fields */
ChardevBackendKind type;
union {
ChardevFile *file; ...
} u;
};
Here, you can pass ChardevBackend* directly (and access
backend->logname, regardless of which union branch is in use).
>> So, depending on which of those three places we want to stick the new
>> parameters determines which approach we should use for exposing them in
>> qapi.
>
>>From the QMP representation POV, my preference is for the current
> design since I think the 'logfile' attribute is really just another
> one of the backend config attributes. The choice to have a ChardevCommon
> struct was just a mechanism to avoid having to repeat the 'logfile'
> parameter in every single Chardev* backend type. This naturally
> matches the C-struct, where the ChardevCommon fields get directly
> added to the ChardevFile, ChardevSocket, etc structs.
Yep - the decision is up to you whether to add it to each struct used as
a branch of ChardevBackend (and each caller then upcasts and passes a
ChardevCommon* to the common code), or whether to add it directly to
ChardevBackend (and each caller merely passes a ChardevBackend*).
>
> So from that POV, I'd be against, pushing the 'logfile' field up
> either 1 or 2 levels.
>
> Regards,
> Daniel
>
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] Request for Help (Generate Trace for Individual Instructions)
Junaid Aslam writes: > Dear Sir > I am a student in Netherlands TU/e and intend to explore QEMU for a project. I > need help in understanding how i can trace an individual instruction which is > translated by TCG. For this moment in am more interested in Guest load store > and > Function call instructions. Can you please help me on how i can generate Trace > file for above mentioned instructions.? I hope to have your reply soon. Not all pieces are merged upstream to make it easy, but you can already add your own trace events for that. Look at "docs/tracing.txt" for info on the "tcg" event property to trace guest instructions. You can then edit the instruction translation file (e.g., "target-i386/translate.c") to insert calls to your new events. Cheers, Lluis
Re: [Qemu-devel] [PATCH v2] qemu-char: add logfile facility to all chardev backends
On 12/23/2015 04:32 AM, Daniel P. Berrange wrote:
> On Tue, Dec 22, 2015 at 12:07:03PM -0700, Eric Blake wrote:
>> On 12/22/2015 11:17 AM, Daniel P. Berrange wrote:
>>> Typically a UNIX guest OS will log boot messages to a serial
>>> port in addition to any graphical console. An admin user
>>> may also wish to use the serial port for an interactive
>>> console. A virtualization management system may wish to
>>> collect system boot messages by logging the serial port,
>>> but also wish to allow admins interactive access.
>>>
>>
>> [code review, if we go with this design; see my other message for 2
>> possible alternative qapi designs that may supersede this code review]
>>
>>> ##
>>> -{ 'struct': 'ChardevDummy', 'data': { } }
>>> +{ 'struct': 'ChardevDummy', 'data': { },
>>> + 'base': 'ChardevCommon' }
>>
>> Instead of changing ChardevDummy, you could have deleted this type and done:
>>
>>>
>>> { 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile',
>>> 'serial' : 'ChardevHostdev',
>> ...
>> 'pty': 'ChardevCommon',
>> 'null': 'ChardevCommon',
>>
>> and so on. I don't know which is better.
>
> Yep, me neither. Given the name it felt like some kind of placeholder
> for future work, so I left it alone.
ChardevDummy exists because we have no way (yet) to represent an empty
type as a union branch. That is, since you can't do:
'pty': {},
we had to instead create a named empty type. But now that we have a
non-empty type, I see no real reason to keep the name, and no reason to
have a subclass that adds no additional fields; so dropping ChardevDummy
is my recommendation.
>>
>> The other thing we could do here is have qemu_chr_open_common() take a
>> ChardevCommon* instead of a ChardevBackend*. Then each caller would do
>> an appropriate upcast before calling the common code:
>>
>> ChardevCommon *common = qapi_ChardevDummy_base(backend->u.null);
>> if (qemu_chr_open_common(chr, common, errp) {
>
> Yep, I think this is the easiest thing todo, to avoid use of
> backend->u.data.
>
>> and so forth. That feels a bit more type-safe (and less reliant on qapi
>> layout guarantees) than trying to rely on the backend->u.data that I'm
>> trying to get rid of.
>
> Agreed
>
Okay, I think having each branch be a subclass of ChardevCommon (and not
ChardevBackend using ChardevCommon as a base) sounds like the way to go,
and now it's up to v3 to rework the clients to be a bit more typesafe.
>>> +++ b/qemu-options.hx
>>> @@ -2034,40 +2034,43 @@ The general form of a character device option is:
>>> ETEXI
>>>
>>> DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
>>> -"-chardev null,id=id[,mux=on|off]\n"
>>> +"-chardev null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n"
>>
>> Do we want to allow logappend=on even when logfile is unspecified, or
>> should we make that an error?
>
> I figured it was harmless to just ignore it.
Works for me.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v2 13/14] qapi: Support pretty printing in JSON output visitor
On 12/23/2015 02:24 AM, Fam Zheng wrote: > On Mon, 12/21 17:31, Eric Blake wrote: >> Similar to pretty printing in the QObject visitor. The rickiest > > "trickiest"? Yep. Fixed in my local tree. > >> parts are the fact that during type_any(), we have to coordinate >> with QObject to also print pretty; and the fact that the testsuite >> now has to honor parameterization on whether pretty printing is >> enabled. >> >> Signed-off-by: Eric Blake > -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v2 0/3] virtio: cross-endian helpers fixes
On Wed, 23 Dec 2015 15:47:00 +0200
"Michael S. Tsirkin" wrote:
> On Thu, Dec 17, 2015 at 09:52:46AM +0100, Greg Kurz wrote:
> > This series tries to rework cross-endian helpers for better clarity.
> > It does not change behaviour, except perhaps patch 3/3 even if I could not
> > measure any performance gain.
>
> Breaks build:
>
> CCmips64-softmmu/hw/mips/mips_malta.o
> /home/mst/scm/qemu/hw/net/vhost_net.c: In function
> ‘vhost_net_set_vnet_endian’:
> /home/mst/scm/qemu/hw/net/vhost_net.c:208:10: error: implicit
> declaration of function ‘virtio_legacy_is_cross_endian’
> [-Werror=implicit-function-declaration]
> (virtio_legacy_is_cross_endian(dev) &&
> !virtio_is_big_endian(dev))) {
> ^
> /home/mst/scm/qemu/hw/net/vhost_net.c:208:9: error: nested extern
> declaration of ‘virtio_legacy_is_cross_endian’ [-Werror=nested-externs]
> (virtio_legacy_is_cross_endian(dev) &&
> !virtio_is_big_endian(dev))) {
> ^
> cc1: all warnings being treated as errors
> /home/mst/scm/qemu/rules.mak:57: recipe for target 'hw/net/vhost_net.o'
> failed
> make[1]: *** [hw/net/vhost_net.o] Error 1
> Makefile:186: recipe for target 'subdir-i386-softmmu' failed
> make: *** [subdir-i386-softmmu] Error 2
>
>
> please always build all architectures.
>
Ok. I'll do so from now on.
> > ---
> >
> > Greg Kurz (3):
> > virtio: move cross-endian helper to vhost
> > vhost: move virtio 1.0 check to cross-endian helper
> > virtio: optimize virtio_access_is_big_endian() for little-endian
> > targets
> >
> >
> > hw/virtio/vhost.c | 22 ++
> > include/hw/virtio/virtio-access.h | 16 +++-
> > 2 files changed, 21 insertions(+), 17 deletions(-)
>
Re: [Qemu-devel] [PATCH v8 15/35] qom: Swap 'name' next to visitor in ObjectPropertyAccessor
On 12/21/2015 10:08 AM, Eric Blake wrote: > Similar to the previous patch, it's nice to have all functions > n the tree that involve a visitor and a name for conversion to s/^n/in/ > or from QAPI to consistently stick the 'name' parameter next > to the Visitor parameter. > -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 7/7] qemu-img: allow specifying image as a set of options args
On Tue, Dec 22, 2015 at 11:10:27AM -0700, Eric Blake wrote: > On 12/22/2015 11:07 AM, Daniel P. Berrange wrote: > > > A third option would be to keep using positional arguments, but > > add a '--source-opts' *boolean* flag to indicate how to interpret > > the positional arguments. ie without --source-opts we use the > > historic syntax, but with --source-opts, we assume the full QemuOpts > > syntax. > > Oh, nice compromise. It's relatively discoverable (grep --help output), > preserves back-compat of old scripts, and offers the full power for > clients that want the full power. I've implemented this now and it makes the patches s much simpler too, so an added win. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
[Qemu-devel] [PATCH v2 00/10] Make qemu-img/qemu-nbd/qemu-io CLI more flexible
This series of patches expands the syntax of the qemu-img, qemu-nbd and qemu-io commands to make them more flexible. v0: http://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04365.html v1: https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg04014.html First all three gain a --object parameter, which allows instantiation of user creatable object types. The immediate use case is to allow for creation of the 'secret' object type to pass passwords for curl, iscsi and rbd drivers. For qemu-nbd this will also be needed to create TLS certificates for encryption support. Then all three gain a '--image-opts' parameter which causes the positional filenames to be interepreted as option strings rather tha nplain filenames. This avoids the need to use the JSON syntax, or to add custom CLI args for each block backend option that exists. The immediate use case is to allow the user to specify the ID of the 'secret' object they just created. Changed in v2: - Share more common code in qom/object_interfaces.c to avoid duplicating so much of 'object_create' in each command - Remove previously added '--source optstring' parameter which replaced the positional filenames, in favour of keeping the positional filenames but using a --image-opts boolean arg to change their interpretation - Added docs for --image-opts to qemu-img man page - Use printf instead of echo -n in examples - Line wrap help string based on user terminal width not source code width - Update qemu-nbd/qemu-io to use constants for options - Update qemu-nbd to avoid overlapping option values Daniel P. Berrange (10): qom: add helpers for UserCreatable object types qemu-img: add support for --object command line arg qemu-nbd: add support for --object command line arg qemu-io: add support for --object command line arg qemu-io: allow specifying image as a set of options args qemu-nbd: allow specifying image as a set of options args qemu-img: allow specifying image as a set of options args qemu-nbd: don't overlap long option values with short options qemu-nbd: use no_argument/required_argument constants qemu-io: use no_argument/required_argument constants hmp.c | 52 +--- include/monitor/monitor.h | 3 - include/qom/object_interfaces.h | 48 qemu-img-cmds.hx| 44 ++-- qemu-img.c | 570 +--- qemu-img.texi | 14 + qemu-io.c | 113 +++- qemu-nbd.c | 148 --- qemu-nbd.texi | 6 + qmp.c | 76 +- qom/object_interfaces.c | 139 ++ vl.c| 48 +--- 12 files changed, 1008 insertions(+), 253 deletions(-) -- 2.5.0
[Qemu-devel] [PATCH v2 03/10] qemu-nbd: add support for --object command line arg
Allow creation of user creatable object types with qemu-nbd
via a new --object command line arg. This will be used to supply
passwords and/or encryption keys to the various block driver
backends via the recently added 'secret' object type.
# printf letmein > mypasswd.txt
# qemu-nbd --object secret,id=sec0,file=mypasswd.txt \
...other nbd args...
Signed-off-by: Daniel P. Berrange
---
qemu-nbd.c| 53 +
qemu-nbd.texi | 6 ++
2 files changed, 59 insertions(+)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 65dc30c..6f97c07 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -23,9 +23,12 @@
#include "qemu/main-loop.h"
#include "qemu/sockets.h"
#include "qemu/error-report.h"
+#include "qemu/config-file.h"
#include "block/snapshot.h"
#include "qapi/util.h"
#include "qapi/qmp/qstring.h"
+#include "qapi/opts-visitor.h"
+#include "qom/object_interfaces.h"
#include
#include
@@ -45,6 +48,7 @@
#define QEMU_NBD_OPT_AIO 2
#define QEMU_NBD_OPT_DISCARD 3
#define QEMU_NBD_OPT_DETECT_ZEROES 4
+#define QEMU_NBD_OPT_OBJECT5
static NBDExport *exp;
static int verbose;
@@ -78,6 +82,9 @@ static void usage(const char *name)
" -o, --offset=OFFSET offset into the image\n"
" -P, --partition=NUM only expose partition NUM\n"
"\n"
+"General purpose options:\n"
+" --object type,id=ID,... define an object such as 'secret' for providing\n"
+"passwords and/or encryption keys\n"
#ifdef __linux__
"Kernel NBD client support:\n"
" -c, --connect=DEV connect FILE to the local NBD device DEV\n"
@@ -380,6 +387,35 @@ static SocketAddress *nbd_build_socket_address(const char
*sockpath,
}
+static QemuOptsList qemu_object_opts = {
+.name = "object",
+.implied_opt_name = "qom-type",
+.head = QTAILQ_HEAD_INITIALIZER(qemu_object_opts.head),
+.desc = {
+{ }
+},
+};
+
+static int object_create(void *opaque, QemuOpts *opts, Error **errp)
+{
+Error *err = NULL;
+OptsVisitor *ov;
+QDict *pdict;
+
+ov = opts_visitor_new(opts);
+pdict = qemu_opts_to_qdict(opts, NULL);
+
+user_creatable_add(pdict, opts_get_visitor(ov), &err);
+opts_visitor_cleanup(ov);
+QDECREF(pdict);
+
+if (err) {
+error_propagate(errp, err);
+return -1;
+}
+return 0;
+}
+
int main(int argc, char **argv)
{
BlockBackend *blk;
@@ -417,6 +453,7 @@ int main(int argc, char **argv)
{ "format", 1, NULL, 'f' },
{ "persistent", 0, NULL, 't' },
{ "verbose", 0, NULL, 'v' },
+{ "object", 1, NULL, QEMU_NBD_OPT_OBJECT },
{ NULL, 0, NULL, 0 }
};
int ch;
@@ -434,6 +471,7 @@ int main(int argc, char **argv)
Error *local_err = NULL;
BlockdevDetectZeroesOptions detect_zeroes =
BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF;
QDict *options = NULL;
+QemuOpts *opts;
/* The client thread uses SIGTERM to interrupt the server. A signal
* handler ensures that "qemu-nbd -v -c" exits with a nice status code.
@@ -442,6 +480,8 @@ int main(int argc, char **argv)
memset(&sa_sigterm, 0, sizeof(sa_sigterm));
sa_sigterm.sa_handler = termsig_handler;
sigaction(SIGTERM, &sa_sigterm, NULL);
+module_call_init(MODULE_INIT_QOM);
+qemu_add_opts(&qemu_object_opts);
qemu_init_exec_dir(argv[0]);
while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
@@ -578,6 +618,13 @@ int main(int argc, char **argv)
usage(argv[0]);
exit(0);
break;
+case QEMU_NBD_OPT_OBJECT:
+opts = qemu_opts_parse_noisily(qemu_find_opts("object"),
+ optarg, true);
+if (!opts) {
+exit(1);
+}
+break;
case '?':
errx(EXIT_FAILURE, "Try `%s --help' for more information.",
argv[0]);
@@ -590,6 +637,12 @@ int main(int argc, char **argv)
argv[0]);
}
+if (qemu_opts_foreach(qemu_find_opts("object"),
+ object_create,
+ NULL, NULL)) {
+exit(1);
+}
+
if (disconnect) {
fd = open(argv[optind], O_RDWR);
if (fd < 0) {
diff --git a/qemu-nbd.texi b/qemu-nbd.texi
index 46fd483..9f9daca 100644
--- a/qemu-nbd.texi
+++ b/qemu-nbd.texi
@@ -14,6 +14,12 @@ Export QEMU disk image using NBD protocol.
@table @option
@item @var{filename}
is a disk image filename
+@item --object type,id=@var{id},...props...
+ define a new instance of the @var{type} object class identified by @var{id}.
+ See the @code{qemu(1)} manual page for full details of the properties
+ supported. The common object type that it makes sense to define is the
+ @code{secret} object, which is used to supply passwords and/or encryption
+ keys.
@item -p, --port=@var{port}
port to listen on (default @samp{10809})
@item -o, --offset=@var{offset}
-
[Qemu-devel] [PATCH v2 06/10] qemu-nbd: allow specifying image as a set of options args
Currently qemu-nbd allows an image filename to be passed on the
command line, but unless using the JSON format, it does not have
a way to set any options except the format eg
qemu-nbd https://127.0.0.1/images/centos7.iso
qemu-nbd /home/berrange/demo.qcow2
This adds a --image-opts arg that indicates that the positional
filename should be interpreted as a full option string, not
just a filename.
qemu-nbd --image-opts driver=http,url=https://127.0.0.1/images,sslverify=off
qemu-nbd --image-opts file=/home/berrange/demo.qcow2
This flag is mutually exclusive with the '-f' flag.
Signed-off-by: Daniel P. Berrange
---
qemu-nbd.c | 44 +++-
1 file changed, 39 insertions(+), 5 deletions(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 6f97c07..02fdcf1 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -49,6 +49,7 @@
#define QEMU_NBD_OPT_DISCARD 3
#define QEMU_NBD_OPT_DETECT_ZEROES 4
#define QEMU_NBD_OPT_OBJECT5
+#define QEMU_NBD_OPT_IMAGE_OPTS6
static NBDExport *exp;
static int verbose;
@@ -387,6 +388,16 @@ static SocketAddress *nbd_build_socket_address(const char
*sockpath,
}
+static QemuOptsList file_opts = {
+.name = "file",
+.implied_opt_name = "file",
+.head = QTAILQ_HEAD_INITIALIZER(file_opts.head),
+.desc = {
+/* no elements => accept any params */
+{ /* end of list */ }
+},
+};
+
static QemuOptsList qemu_object_opts = {
.name = "object",
.implied_opt_name = "qom-type",
@@ -454,6 +465,7 @@ int main(int argc, char **argv)
{ "persistent", 0, NULL, 't' },
{ "verbose", 0, NULL, 'v' },
{ "object", 1, NULL, QEMU_NBD_OPT_OBJECT },
+{ "image-opts", 0, NULL, QEMU_NBD_OPT_IMAGE_OPTS },
{ NULL, 0, NULL, 0 }
};
int ch;
@@ -472,6 +484,7 @@ int main(int argc, char **argv)
BlockdevDetectZeroesOptions detect_zeroes =
BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF;
QDict *options = NULL;
QemuOpts *opts;
+bool imageOpts = false;
/* The client thread uses SIGTERM to interrupt the server. A signal
* handler ensures that "qemu-nbd -v -c" exits with a nice status code.
@@ -625,6 +638,9 @@ int main(int argc, char **argv)
exit(1);
}
break;
+case QEMU_NBD_OPT_IMAGE_OPTS:
+imageOpts = true;
+break;
case '?':
errx(EXIT_FAILURE, "Try `%s --help' for more information.",
argv[0]);
@@ -725,13 +741,31 @@ int main(int argc, char **argv)
bdrv_init();
atexit(bdrv_close_all);
-if (fmt) {
-options = qdict_new();
-qdict_put(options, "driver", qstring_from_str(fmt));
+srcpath = argv[optind];
+if (imageOpts) {
+char *file = NULL;
+if (fmt) {
+errx(EXIT_FAILURE, "--image-opts and -f are mutually exclusive");
+}
+opts = qemu_opts_parse_noisily(&file_opts, srcpath, true);
+if (!opts) {
+qemu_opts_reset(&file_opts);
+exit(EXIT_FAILURE);
+}
+file = g_strdup(qemu_opt_get(opts, "file"));
+qemu_opt_unset(opts, "file");
+options = qemu_opts_to_qdict(opts, NULL);
+qemu_opts_reset(&file_opts);
+blk = blk_new_open("hda", file, NULL, options, flags, &local_err);
+g_free(file);
+} else {
+if (fmt) {
+options = qdict_new();
+qdict_put(options, "driver", qstring_from_str(fmt));
+}
+blk = blk_new_open("hda", srcpath, NULL, options, flags, &local_err);
}
-srcpath = argv[optind];
-blk = blk_new_open("hda", srcpath, NULL, options, flags, &local_err);
if (!blk) {
errx(EXIT_FAILURE, "Failed to blk_new_open '%s': %s", argv[optind],
error_get_pretty(local_err));
--
2.5.0
[Qemu-devel] [PATCH v2 01/10] qom: add helpers for UserCreatable object types
The QMP monitor code has two helper methods object_add
and qmp_object_del that are called from several places
in the code (QMP, HMP and main emulator startup).
The HMP and main emulator startup code also share
further logic that extracts the qom-type & id
values from a qdict.
We soon need to use this logic from qemu-img, qemu-io
and qemu-nbd too, but don't want those to depend on
the monitor, nor do we want to duplicate the code.
To avoid this, move some code out of qmp.c and hmp.c
adding 3 new methods to qom/object_interfaces.c
- user_creatable_add - takes a QDict holding a full
object definition & instantiates it
- user_creatable_add_type - takes an ID, type name,
and QDict holding object properties & instantiates
it
- user_creatable_del - takes an ID and deletes the
corresponding object
The existing code is updated to use these new methods.
Signed-off-by: Daniel P. Berrange
---
hmp.c | 52 ---
include/monitor/monitor.h | 3 -
include/qom/object_interfaces.h | 48 ++
qmp.c | 76 ++
qom/object_interfaces.c | 139
vl.c| 48 --
6 files changed, 216 insertions(+), 150 deletions(-)
diff --git a/hmp.c b/hmp.c
index c2b2c16..6a9c51d 100644
--- a/hmp.c
+++ b/hmp.c
@@ -29,6 +29,7 @@
#include "qapi/string-output-visitor.h"
#include "qapi/util.h"
#include "qapi-visit.h"
+#include "qom/object_interfaces.h"
#include "ui/console.h"
#include "block/qapi.h"
#include "qemu-io.h"
@@ -1663,58 +1664,27 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
void hmp_object_add(Monitor *mon, const QDict *qdict)
{
Error *err = NULL;
-Error *err_end = NULL;
QemuOpts *opts;
-char *type = NULL;
-char *id = NULL;
-void *dummy = NULL;
OptsVisitor *ov;
-QDict *pdict;
+Object *obj = NULL;
opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, &err);
if (err) {
-goto out;
+hmp_handle_error(mon, &err);
+return;
}
ov = opts_visitor_new(opts);
-pdict = qdict_clone_shallow(qdict);
-
-visit_start_struct(opts_get_visitor(ov), &dummy, NULL, NULL, 0, &err);
-if (err) {
-goto out_clean;
-}
-
-qdict_del(pdict, "qom-type");
-visit_type_str(opts_get_visitor(ov), &type, "qom-type", &err);
-if (err) {
-goto out_end;
-}
+obj = user_creatable_add(qdict, opts_get_visitor(ov), &err);
+opts_visitor_cleanup(ov);
+qemu_opts_del(opts);
-qdict_del(pdict, "id");
-visit_type_str(opts_get_visitor(ov), &id, "id", &err);
if (err) {
-goto out_end;
+hmp_handle_error(mon, &err);
}
-
-object_add(type, id, pdict, opts_get_visitor(ov), &err);
-
-out_end:
-visit_end_struct(opts_get_visitor(ov), &err_end);
-if (!err && err_end) {
-qmp_object_del(id, NULL);
+if (obj) {
+object_unref(obj);
}
-error_propagate(&err, err_end);
-out_clean:
-opts_visitor_cleanup(ov);
-
-QDECREF(pdict);
-qemu_opts_del(opts);
-g_free(id);
-g_free(type);
-g_free(dummy);
-
-out:
-hmp_handle_error(mon, &err);
}
void hmp_getfd(Monitor *mon, const QDict *qdict)
@@ -1944,7 +1914,7 @@ void hmp_object_del(Monitor *mon, const QDict *qdict)
const char *id = qdict_get_str(qdict, "id");
Error *err = NULL;
-qmp_object_del(id, &err);
+user_creatable_del(id, &err);
hmp_handle_error(mon, &err);
}
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 91b95ae..aa0f373 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -43,9 +43,6 @@ void monitor_read_command(Monitor *mon, int show_prompt);
int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
void *opaque);
-void object_add(const char *type, const char *id, const QDict *qdict,
-Visitor *v, Error **errp);
-
AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
bool has_opaque, const char *opaque,
Error **errp);
diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index 283ae0d..7bbaf2f 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -2,6 +2,8 @@
#define OBJECT_INTERFACES_H
#include "qom/object.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/visitor.h"
#define TYPE_USER_CREATABLE "user-creatable"
@@ -72,4 +74,50 @@ void user_creatable_complete(Object *obj, Error **errp);
* from implements USER_CREATABLE interface.
*/
bool user_creatable_can_be_deleted(UserCreatable *uc, Error **errp);
+
+/**
+ * user_creatable_add:
+ * @qdict: the object definition
+ * @v: the visitor
+ * @errp: if an error occurs, a pointer to an area to store the error
+ *
+ * Create an instance of the user creatable ob
[Qemu-devel] [PATCH v2 04/10] qemu-io: add support for --object command line arg
Allow creation of user creatable object types with qemu-io
via a new --object command line arg. This will be used to supply
passwords and/or encryption keys to the various block driver
backends via the recently added 'secret' object type.
# printf letmein > mypasswd.txt
# qemu-io --object secret,id=sec0,file=mypasswd.txt \
...other args...
Signed-off-by: Daniel P. Berrange
---
qemu-io.c | 53 +
1 file changed, 53 insertions(+)
diff --git a/qemu-io.c b/qemu-io.c
index 269f17c..eedab73 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -21,6 +21,8 @@
#include "qemu/config-file.h"
#include "qemu/readline.h"
#include "qapi/qmp/qstring.h"
+#include "qapi/opts-visitor.h"
+#include "qom/object_interfaces.h"
#include "sysemu/block-backend.h"
#include "block/block_int.h"
#include "trace/control.h"
@@ -205,6 +207,8 @@ static void usage(const char *name)
"Usage: %s [-h] [-V] [-rsnm] [-f FMT] [-c STRING] ... [file]\n"
"QEMU Disk exerciser\n"
"\n"
+" --object OBJECTDEF define an object such as 'secret' for\n"
+" passwords and/or encryption keys\n"
" -c, --cmd STRING execute command with its arguments\n"
" from the given string\n"
" -f, --format FMT specifies the block driver to use\n"
@@ -366,6 +370,38 @@ static void reenable_tty_echo(void)
qemu_set_tty_echo(STDIN_FILENO, true);
}
+enum {
+OPTION_OBJECT = 256,
+};
+
+static QemuOptsList qemu_object_opts = {
+.name = "object",
+.implied_opt_name = "qom-type",
+.head = QTAILQ_HEAD_INITIALIZER(qemu_object_opts.head),
+.desc = {
+{ }
+},
+};
+
+static int object_create(void *opaque, QemuOpts *opts, Error **errp)
+{
+Error *err = NULL;
+OptsVisitor *ov;
+QDict *pdict;
+
+ov = opts_visitor_new(opts);
+pdict = qemu_opts_to_qdict(opts, NULL);
+
+user_creatable_add(pdict, opts_get_visitor(ov), &err);
+opts_visitor_cleanup(ov);
+QDECREF(pdict);
+if (err) {
+error_propagate(errp, err);
+return -1;
+}
+return 0;
+}
+
int main(int argc, char **argv)
{
int readonly = 0;
@@ -384,6 +420,7 @@ int main(int argc, char **argv)
{ "discard", 1, NULL, 'd' },
{ "cache", 1, NULL, 't' },
{ "trace", 1, NULL, 'T' },
+{ "object", 1, NULL, OPTION_OBJECT },
{ NULL, 0, NULL, 0 }
};
int c;
@@ -391,6 +428,7 @@ int main(int argc, char **argv)
int flags = BDRV_O_UNMAP;
Error *local_error = NULL;
QDict *opts = NULL;
+QemuOpts *qopts = NULL;
#ifdef CONFIG_POSIX
signal(SIGPIPE, SIG_IGN);
@@ -399,6 +437,8 @@ int main(int argc, char **argv)
progname = basename(argv[0]);
qemu_init_exec_dir(argv[0]);
+module_call_init(MODULE_INIT_QOM);
+qemu_add_opts(&qemu_object_opts);
bdrv_init();
while ((c = getopt_long(argc, argv, sopt, lopt, &opt_index)) != -1) {
@@ -450,6 +490,13 @@ int main(int argc, char **argv)
case 'h':
usage(progname);
exit(0);
+case OPTION_OBJECT:
+qopts = qemu_opts_parse_noisily(qemu_find_opts("object"),
+optarg, true);
+if (!qopts) {
+exit(1);
+}
+break;
default:
usage(progname);
exit(1);
@@ -466,6 +513,12 @@ int main(int argc, char **argv)
exit(1);
}
+if (qemu_opts_foreach(qemu_find_opts("object"),
+ object_create,
+ NULL, NULL)) {
+exit(1);
+}
+
/* initialize commands */
qemuio_add_command(&quit_cmd);
qemuio_add_command(&open_cmd);
--
2.5.0
[Qemu-devel] [PATCH v2 05/10] qemu-io: allow specifying image as a set of options args
Currently qemu-io allows an image filename to be passed on the
command line, but unless using the JSON format, it does not have
a way to set any options except the format eg
qemu-io https://127.0.0.1/images/centos7.iso
qemu-io /home/berrange/demo.qcow2
This adds a --image-opts arg that indicates that the positional
filename should be interpreted as a full option string, not
just a filename.
qemu-io --image-opts driver=http,url=https://127.0.0.1/images,sslverify=off
qemu-io --image-opts file=/home/berrange/demo.qcow2
This flag is mutually exclusive with the '-f' flag.
Signed-off-by: Daniel P. Berrange
---
qemu-io.c | 34 +-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/qemu-io.c b/qemu-io.c
index eedab73..39178f3 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -372,6 +372,7 @@ static void reenable_tty_echo(void)
enum {
OPTION_OBJECT = 256,
+OPTION_IMAGE_OPTS = 257,
};
static QemuOptsList qemu_object_opts = {
@@ -402,6 +403,16 @@ static int object_create(void *opaque, QemuOpts *opts,
Error **errp)
return 0;
}
+static QemuOptsList file_opts = {
+.name = "file",
+.implied_opt_name = "file",
+.head = QTAILQ_HEAD_INITIALIZER(file_opts.head),
+.desc = {
+/* no elements => accept any params */
+{ /* end of list */ }
+},
+};
+
int main(int argc, char **argv)
{
int readonly = 0;
@@ -421,6 +432,7 @@ int main(int argc, char **argv)
{ "cache", 1, NULL, 't' },
{ "trace", 1, NULL, 'T' },
{ "object", 1, NULL, OPTION_OBJECT },
+{ "image-opts", 0, NULL, OPTION_IMAGE_OPTS },
{ NULL, 0, NULL, 0 }
};
int c;
@@ -429,6 +441,7 @@ int main(int argc, char **argv)
Error *local_error = NULL;
QDict *opts = NULL;
QemuOpts *qopts = NULL;
+bool imageOpts = false;
#ifdef CONFIG_POSIX
signal(SIGPIPE, SIG_IGN);
@@ -497,6 +510,9 @@ int main(int argc, char **argv)
exit(1);
}
break;
+case OPTION_IMAGE_OPTS:
+imageOpts = true;
+break;
default:
usage(progname);
exit(1);
@@ -538,7 +554,23 @@ int main(int argc, char **argv)
flags |= BDRV_O_RDWR;
}
-if ((argc - optind) == 1) {
+if (imageOpts) {
+char *file;
+qopts = qemu_opts_parse_noisily(&file_opts, argv[optind], false);
+if (!qopts) {
+exit(1);
+}
+if (opts) {
+error_report("--image-opts and -f are mutually exclusive");
+exit(1);
+}
+file = g_strdup(qemu_opt_get(qopts, "file"));
+qemu_opt_unset(qopts, "file");
+opts = qemu_opts_to_qdict(qopts, NULL);
+qemu_opts_reset(&file_opts);
+openfile(file, flags, opts);
+g_free(file);
+} else if ((argc - optind) == 1) {
openfile(argv[optind], flags, opts);
}
command_loop();
--
2.5.0
[Qemu-devel] [PATCH v2 08/10] qemu-nbd: don't overlap long option values with short options
When defining values for long options, the normal practice is to start numbering from 256, to avoid overlap with the range of valid values for short options. Signed-off-by: Daniel P. Berrange --- qemu-nbd.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/qemu-nbd.c b/qemu-nbd.c index 02fdcf1..9898e22 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -44,12 +44,12 @@ #include #define SOCKET_PATH"/var/lock/qemu-nbd-%s" -#define QEMU_NBD_OPT_CACHE 1 -#define QEMU_NBD_OPT_AIO 2 -#define QEMU_NBD_OPT_DISCARD 3 -#define QEMU_NBD_OPT_DETECT_ZEROES 4 -#define QEMU_NBD_OPT_OBJECT5 -#define QEMU_NBD_OPT_IMAGE_OPTS6 +#define QEMU_NBD_OPT_CACHE 256 +#define QEMU_NBD_OPT_AIO 257 +#define QEMU_NBD_OPT_DISCARD 258 +#define QEMU_NBD_OPT_DETECT_ZEROES 259 +#define QEMU_NBD_OPT_OBJECT260 +#define QEMU_NBD_OPT_IMAGE_OPTS261 static NBDExport *exp; static int verbose; -- 2.5.0
