Re: [PATCH v12 3/7] net/vmnet: implement shared mode (vmnet-shared)
Will fix it, thank you!
Actually this (wrong) check:
#if defined(MAC_OS_VERSION_11_0) && \
MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_VERSION_11_0
worked fine on Catalina 10.15 for me cause the
MAC_OS_VERSION_11_0 was not introduced there and it
stopped on the 1st defined(MAC_OS_VERSION_11_0) check.
I should have tested this more thoroughly, my fault.
Now submitting v13 with MAC_OS_X_VERSION_MIN_REQUIRED.
--
Best Regards,
Vladislav Yaroshchuk
чт, 13 янв. 2022 г. в 19:11, Akihiko Odaki :
> On Fri, Jan 14, 2022 at 12:01 AM Vladislav Yaroshchuk
> wrote:
> >
> > Interaction with vmnet.framework in different modes
> > differs only on configuration stage, so we can create
> > common `send`, `receive`, etc. procedures and reuse them.
> >
> > vmnet.framework supports iov, but writing more than
> > one iov into vmnet interface fails with
> > 'VMNET_INVALID_ARGUMENT'. Collecting provided iovs into
> > one and passing it to vmnet works fine. That's the
> > reason why receive_iov() left unimplemented. But it still
> > works with good enough performance having .receive()
> > implemented only.
> >
> > Also, there is no way to unsubscribe from vmnet packages
> > receiving except registering and unregistering event
> > callback or simply drop packages just ignoring and
> > not processing them when related flag is set. Here we do
> > using the second way.
> >
> > Signed-off-by: Phillip Tennen
> > Signed-off-by: Vladislav Yaroshchuk
> > ---
> > net/vmnet-common.m | 313 +
> > net/vmnet-shared.c | 83 +++-
> > net/vmnet_int.h| 23
> > 3 files changed, 415 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/vmnet-common.m b/net/vmnet-common.m
> > index 532d152840..45c983ac7f 100644
> > --- a/net/vmnet-common.m
> > +++ b/net/vmnet-common.m
> > @@ -10,6 +10,8 @@
> > */
> >
> > #include "qemu/osdep.h"
> > +#include "qemu/main-loop.h"
> > +#include "qemu/log.h"
> > #include "qapi/qapi-types-net.h"
> > #include "vmnet_int.h"
> > #include "clients.h"
> > @@ -17,4 +19,315 @@
> > #include "qapi/error.h"
> >
> > #include
> > +#include
> >
> > +#ifdef DEBUG
> > +#define D(x) x
> > +#define D_LOG(...) qemu_log(__VA_ARGS__)
> > +#else
> > +#define D(x) do { } while (0)
> > +#define D_LOG(...) do { } while (0)
> > +#endif
> > +
> > +typedef struct vmpktdesc vmpktdesc_t;
> > +typedef struct iovec iovec_t;
> > +
> > +static void vmnet_set_send_enabled(VmnetCommonState *s, bool enable)
> > +{
> > +s->send_enabled = enable;
> > +}
> > +
> > +
> > +static void vmnet_send_completed(NetClientState *nc, ssize_t len)
> > +{
> > +VmnetCommonState *s = DO_UPCAST(VmnetCommonState, nc, nc);
> > +vmnet_set_send_enabled(s, true);
> > +}
> > +
> > +
> > +static void vmnet_send(NetClientState *nc,
> > + interface_event_t event_id,
> > + xpc_object_t event)
> > +{
> > +assert(event_id == VMNET_INTERFACE_PACKETS_AVAILABLE);
> > +
> > +VmnetCommonState *s;
> > +uint64_t packets_available;
> > +
> > +struct iovec *iov;
> > +struct vmpktdesc *packets;
> > +int pkt_cnt;
> > +int i;
> > +
> > +vmnet_return_t if_status;
> > +ssize_t size;
> > +
> > +s = DO_UPCAST(VmnetCommonState, nc, nc);
> > +
> > +packets_available = xpc_dictionary_get_uint64(
> > +event,
> > +vmnet_estimated_packets_available_key
> > +);
> > +
> > +pkt_cnt = (packets_available < VMNET_PACKETS_LIMIT) ?
> > + packets_available :
> > + VMNET_PACKETS_LIMIT;
> > +
> > +
> > +iov = s->iov_buf;
> > +packets = s->packets_buf;
> > +
> > +for (i = 0; i < pkt_cnt; ++i) {
> > +packets[i].vm_pkt_size = s->max_packet_size;
> > +packets[i].vm_pkt_iovcnt = 1;
> > +packets[i].vm_flags = 0;
> > +}
> > +
> > +if_status = vmnet_read(s->vmnet_if, packets, _cnt);
> > +if (if_status != VMNET_SUCCESS) {
> > +error_printf("vmnet: read failed: %s\n",
> > + vmnet_status_map_str(if_status));
> > +}
> > +qemu_mutex_lock_iothread();
> > +for (i = 0; i < pkt_cnt; ++i) {
> > +size = qemu_send_packet_async(nc,
> > + iov[i].iov_base,
> > + packets[i].vm_pkt_size,
> > + vmnet_send_completed);
> > +if (size == 0) {
> > +vmnet_set_send_enabled(s, false);
> > +} else if (size < 0) {
> > +break;
> > +}
> > +}
> > +qemu_mutex_unlock_iothread();
> > +
> > +}
> > +
> > +
> > +static void vmnet_register_event_callback(VmnetCommonState *s)
> > +{
> > +dispatch_queue_t avail_pkt_q = dispatch_queue_create(
> > +"org.qemu.vmnet.if_queue",
> > +DISPATCH_QUEUE_SERIAL
> > +);
> > +
> > +vmnet_interface_set_event_callback(
> > +s->vmnet_if,
> > +VMNET_INTERFACE_PACKETS_AVAILABLE,
> > +avail_pkt_q,
Re: [PATCH] iotests/testrunner.py: refactor test_field_width
Am 10.12.2021 um 21:14 hat Vladimir Sementsov-Ogievskiy geschrieben: > A lot of Optional[] types doesn't make code beautiful. > test_field_width defaults to 8, but that is never used in the code. > > More over, if we want some default behavior for single call of > test_run(), it should just print the whole test name, not limiting or > expanding its width, so 8 is bad default. > > So, just drop the default as unused for now. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Thanks, applied to the block branch. Kevin
Re: [PATCH v5 00/19] iotests: support zstd
On 23.12.21 17:01, Vladimir Sementsov-Ogievskiy wrote: These series makes tests pass with IMGOPTS='compression_type=zstd' Also, python iotests start to support IMGOPTS (they didn't before). v5: Move patches with unsupported_imgopts to the start of the series 02: add Hanna's r-b 03: - don't modify migrate-during-backup test - disably any 'compat' instead of just 'compat=0.10' Thanks! Applied to my block branch: https://gitlab.com/hreitz/qemu/-/commits/block
Re: [PATCH v12 3/7] net/vmnet: implement shared mode (vmnet-shared)
On Fri, Jan 14, 2022 at 12:01 AM Vladislav Yaroshchuk
wrote:
>
> Interaction with vmnet.framework in different modes
> differs only on configuration stage, so we can create
> common `send`, `receive`, etc. procedures and reuse them.
>
> vmnet.framework supports iov, but writing more than
> one iov into vmnet interface fails with
> 'VMNET_INVALID_ARGUMENT'. Collecting provided iovs into
> one and passing it to vmnet works fine. That's the
> reason why receive_iov() left unimplemented. But it still
> works with good enough performance having .receive()
> implemented only.
>
> Also, there is no way to unsubscribe from vmnet packages
> receiving except registering and unregistering event
> callback or simply drop packages just ignoring and
> not processing them when related flag is set. Here we do
> using the second way.
>
> Signed-off-by: Phillip Tennen
> Signed-off-by: Vladislav Yaroshchuk
> ---
> net/vmnet-common.m | 313 +
> net/vmnet-shared.c | 83 +++-
> net/vmnet_int.h| 23
> 3 files changed, 415 insertions(+), 4 deletions(-)
>
> diff --git a/net/vmnet-common.m b/net/vmnet-common.m
> index 532d152840..45c983ac7f 100644
> --- a/net/vmnet-common.m
> +++ b/net/vmnet-common.m
> @@ -10,6 +10,8 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/main-loop.h"
> +#include "qemu/log.h"
> #include "qapi/qapi-types-net.h"
> #include "vmnet_int.h"
> #include "clients.h"
> @@ -17,4 +19,315 @@
> #include "qapi/error.h"
>
> #include
> +#include
>
> +#ifdef DEBUG
> +#define D(x) x
> +#define D_LOG(...) qemu_log(__VA_ARGS__)
> +#else
> +#define D(x) do { } while (0)
> +#define D_LOG(...) do { } while (0)
> +#endif
> +
> +typedef struct vmpktdesc vmpktdesc_t;
> +typedef struct iovec iovec_t;
> +
> +static void vmnet_set_send_enabled(VmnetCommonState *s, bool enable)
> +{
> +s->send_enabled = enable;
> +}
> +
> +
> +static void vmnet_send_completed(NetClientState *nc, ssize_t len)
> +{
> +VmnetCommonState *s = DO_UPCAST(VmnetCommonState, nc, nc);
> +vmnet_set_send_enabled(s, true);
> +}
> +
> +
> +static void vmnet_send(NetClientState *nc,
> + interface_event_t event_id,
> + xpc_object_t event)
> +{
> +assert(event_id == VMNET_INTERFACE_PACKETS_AVAILABLE);
> +
> +VmnetCommonState *s;
> +uint64_t packets_available;
> +
> +struct iovec *iov;
> +struct vmpktdesc *packets;
> +int pkt_cnt;
> +int i;
> +
> +vmnet_return_t if_status;
> +ssize_t size;
> +
> +s = DO_UPCAST(VmnetCommonState, nc, nc);
> +
> +packets_available = xpc_dictionary_get_uint64(
> +event,
> +vmnet_estimated_packets_available_key
> +);
> +
> +pkt_cnt = (packets_available < VMNET_PACKETS_LIMIT) ?
> + packets_available :
> + VMNET_PACKETS_LIMIT;
> +
> +
> +iov = s->iov_buf;
> +packets = s->packets_buf;
> +
> +for (i = 0; i < pkt_cnt; ++i) {
> +packets[i].vm_pkt_size = s->max_packet_size;
> +packets[i].vm_pkt_iovcnt = 1;
> +packets[i].vm_flags = 0;
> +}
> +
> +if_status = vmnet_read(s->vmnet_if, packets, _cnt);
> +if (if_status != VMNET_SUCCESS) {
> +error_printf("vmnet: read failed: %s\n",
> + vmnet_status_map_str(if_status));
> +}
> +qemu_mutex_lock_iothread();
> +for (i = 0; i < pkt_cnt; ++i) {
> +size = qemu_send_packet_async(nc,
> + iov[i].iov_base,
> + packets[i].vm_pkt_size,
> + vmnet_send_completed);
> +if (size == 0) {
> +vmnet_set_send_enabled(s, false);
> +} else if (size < 0) {
> +break;
> +}
> +}
> +qemu_mutex_unlock_iothread();
> +
> +}
> +
> +
> +static void vmnet_register_event_callback(VmnetCommonState *s)
> +{
> +dispatch_queue_t avail_pkt_q = dispatch_queue_create(
> +"org.qemu.vmnet.if_queue",
> +DISPATCH_QUEUE_SERIAL
> +);
> +
> +vmnet_interface_set_event_callback(
> +s->vmnet_if,
> +VMNET_INTERFACE_PACKETS_AVAILABLE,
> +avail_pkt_q,
> +^(interface_event_t event_id, xpc_object_t event) {
> + if (s->send_enabled) {
> + vmnet_send(>nc, event_id, event);
> + }
> +});
> +}
> +
> +
> +static void vmnet_bufs_init(VmnetCommonState *s)
> +{
> +int i;
> +struct vmpktdesc *packets;
> +struct iovec *iov;
> +
> +packets = s->packets_buf;
> +iov = s->iov_buf;
> +
> +for (i = 0; i < VMNET_PACKETS_LIMIT; ++i) {
> +iov[i].iov_len = s->max_packet_size;
> +iov[i].iov_base = g_malloc0(iov[i].iov_len);
> +packets[i].vm_pkt_iov = iov + i;
> +}
> +}
> +
> +
> +const char *vmnet_status_map_str(vmnet_return_t status)
> +{
> +switch (status) {
> +case VMNET_SUCCESS:
> +return "success";
> +case VMNET_FAILURE:
> +
Re: [PATCH] iotests/MRCE: Write data to source
On 23.12.21 17:53, Hanna Reitz wrote: This test assumes that mirror flushes the source when entering the READY state, and that the format level will pass that flush on to the protocol level (where we intercept it with blkdebug). However, apparently that does not happen when using a VMDK image with zeroed_grain=on, which actually is the default set by testenv.py. Right now, Python tests ignore IMGOPTS, though, so this has no effect; but Vladimir has a series that will change this, so we need to fix this test before that series lands. We can fix it by writing data to the source before we start the mirror job; apparently that makes the (VMDK) format layer change its mind and pass on the pre-READY flush to the protocol level, so the test passes again. (I presume, without any data written, mirror just does a 64M zero write on the target, which VMDK with zeroed_grain=on basically just ignores.) Without this, we do not get a flush, and so blkdebug only sees a single flush at the end of the job instead of two, and therefore does not inject an error, which makes the block job complete instead of raising an error. Signed-off-by: Hanna Reitz --- As hinted at above, I think this should be merged before Vladimir's "iotests: support zstd" series, or said test fails for me with VMDK. (At least on one system, not the other... Would be too easy otherwise, obviously.) --- tests/qemu-iotests/tests/mirror-ready-cancel-error | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) Applied to my block branch. Hanna
[PATCH V2 for-6.2 1/2] block/rbd: fix handling of holes in .bdrv_co_block_status
the assumption that we can't hit a hole if we do not diff against a snapshot
was wrong.
We can see a hole in an image if we diff against base if there exists an older
snapshot
of the image and we have discarded blocks in the image where the snapshot has
data.
Fix this by simply handling a hole like an unallocated area. There are no
callbacks
for unallocated areas so just bail out if we hit a hole.
Fixes: 0347a8fd4c3faaedf119be04c197804be40a384b
Suggested-by: Ilya Dryomov
Cc: qemu-sta...@nongnu.org
Signed-off-by: Peter Lieven
---
block/rbd.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/block/rbd.c b/block/rbd.c
index def96292e0..20bb896c4a 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -1279,11 +1279,11 @@ static int qemu_rbd_diff_iterate_cb(uint64_t offs,
size_t len,
RBDDiffIterateReq *req = opaque;
assert(req->offs + req->bytes <= offs);
-/*
- * we do not diff against a snapshot so we should never receive a callback
- * for a hole.
- */
-assert(exists);
+
+/* treat a hole like an unallocated area and bail out */
+if (!exists) {
+return 0;
+}
if (!req->exists && offs > req->offs) {
/*
--
2.25.1
[PATCH V2 for-6.2 2/2] block/rbd: workaround for ceph issue #53784
librbd had a bug until early 2022 that affected all versions of ceph that
supported fast-diff. This bug results in reporting of incorrect offsets
if the offset parameter to rbd_diff_iterate2 is not object aligned.
This patch works around this bug for pre Quincy versions of librbd.
Cc: qemu-sta...@nongnu.org
Signed-off-by: Peter Lieven
---
block/rbd.c | 42 --
1 file changed, 40 insertions(+), 2 deletions(-)
diff --git a/block/rbd.c b/block/rbd.c
index 20bb896c4a..d174d51659 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -1320,6 +1320,7 @@ static int coroutine_fn
qemu_rbd_co_block_status(BlockDriverState *bs,
int status, r;
RBDDiffIterateReq req = { .offs = offset };
uint64_t features, flags;
+uint64_t head = 0;
assert(offset + bytes <= s->image_size);
@@ -1347,7 +1348,43 @@ static int coroutine_fn
qemu_rbd_co_block_status(BlockDriverState *bs,
return status;
}
-r = rbd_diff_iterate2(s->image, NULL, offset, bytes, true, true,
+#if LIBRBD_VERSION_CODE < LIBRBD_VERSION(1, 17, 0)
+/*
+ * librbd had a bug until early 2022 that affected all versions of ceph
that
+ * supported fast-diff. This bug results in reporting of incorrect offsets
+ * if the offset parameter to rbd_diff_iterate2 is not object aligned.
+ * Work around this bug by rounding down the offset to object boundaries.
+ * This is OK because we call rbd_diff_iterate2 with whole_object = true.
+ * However, this workaround only works for non cloned images with default
+ * striping.
+ *
+ * See: https://tracker.ceph.com/issues/53784
+ */
+
+/* check if RBD image has non-default striping enabled */
+if (features & RBD_FEATURE_STRIPINGV2) {
+return status;
+}
+
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+/*
+ * check if RBD image is a clone (= has a parent).
+ *
+ * rbd_get_parent_info is deprecated from Nautilus onwards, but the
+ * replacement rbd_get_parent is not present in Luminous and Mimic.
+ */
+if (rbd_get_parent_info(s->image, NULL, 0, NULL, 0, NULL, 0) != -ENOENT) {
+return status;
+}
+#pragma GCC diagnostic pop
+
+head = req.offs & (s->object_size - 1);
+req.offs -= head;
+bytes += head;
+#endif
+
+r = rbd_diff_iterate2(s->image, NULL, req.offs, bytes, true, true,
qemu_rbd_diff_iterate_cb, );
if (r < 0 && r != QEMU_RBD_EXIT_DIFF_ITERATE2) {
return status;
@@ -1366,7 +1403,8 @@ static int coroutine_fn
qemu_rbd_co_block_status(BlockDriverState *bs,
status = BDRV_BLOCK_ZERO | BDRV_BLOCK_OFFSET_VALID;
}
-*pnum = req.bytes;
+assert(req.bytes > head);
+*pnum = req.bytes - head;
return status;
}
--
2.25.1
[PATCH V2 for-6.2 0/2] fixes for bdrv_co_block_status
V1->V2: Patch 1: Treat a hole just like an unallocated area. [Ilya] Patch 2: Apply workaround only for pre-Quincy librbd versions and ensure default striping and non child images. [Ilya] Peter Lieven (2): block/rbd: fix handling of holes in .bdrv_co_block_status block/rbd: workaround for ceph issue #53784 block/rbd.c | 52 +--- 1 file changed, 45 insertions(+), 7 deletions(-) -- 2.25.1
Re: [PATCH] Fix null pointer dereference in util/fdmon-epoll.c
On Tue, Jan 11, 2022 at 08:10:59PM +0800, Daniella Lee wrote:
> Orginal qemu commit hash: de3f5223fa4cf8bfc5e3fe1fd495ddf468edcdf7
> In util/fdmon-epoll.c, function fdmon_epoll_update, variable "old_node"
> maybe NULL with the condition, while it is directly used in the statement and
> may lead to null pointer dereferencen problem.
> Variable "r" in the condition is the return value of epoll_ctl function,
> and will return -1 when failed.
> Therefore, the patch added a check and initialized the variable "r".
>
>
> Signed-off-by: Daniella Lee
> ---
> util/fdmon-epoll.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
Hi Daniella,
Thanks for the patch! How is the new_node == NULL && old_node == NULL
case reached?
The caller is util/aio-posix.c:aio_set_fd_handler():
AioHandler *node;
AioHandler *new_node = NULL;
...
node = find_aio_handler(ctx, fd);
/* Are we deleting the fd handler? */
if (!io_read && !io_write && !io_poll) {
if (node == NULL) {
qemu_lockcnt_unlock(>list_lock);
return; /* old_node == NULL && new_node == NULL */
}
... /* old_node != NULL && new_node == NULL */
} else {
...
new_node = g_new0(AioHandler, 1);
...
}
/* (old_node != NULL && new_node == NULL) || (new_node != NULL) */
...
ctx->fdmon_ops->update(ctx, node, new_node);
aio_set_fd_handler() returns early instead of calling ->update() when
old_node == NULL && new_node == NULL. It looks like the NULL pointer
dereference cannot happen and semantically it doesn't make sense to call
->update(ctx, NULL, NULL) since there is nothing to update so it's
unlikely to be called this way in the future.
Have I missed something?
Thanks,
Stefan
> diff --git a/util/fdmon-epoll.c b/util/fdmon-epoll.c
> index e11a8a022e..3c8b0de694 100644
> --- a/util/fdmon-epoll.c
> +++ b/util/fdmon-epoll.c
> @@ -38,10 +38,12 @@ static void fdmon_epoll_update(AioContext *ctx,
> .data.ptr = new_node,
> .events = new_node ? epoll_events_from_pfd(new_node->pfd.events) : 0,
> };
> -int r;
> +int r = -1;
>
> if (!new_node) {
> -r = epoll_ctl(ctx->epollfd, EPOLL_CTL_DEL, old_node->pfd.fd, );
> +if (old_node) {
> +r = epoll_ctl(ctx->epollfd, EPOLL_CTL_DEL, old_node->pfd.fd,
> );
> +}
> } else if (!old_node) {
> r = epoll_ctl(ctx->epollfd, EPOLL_CTL_ADD, new_node->pfd.fd, );
> } else {
> --
> 2.17.1
>
signature.asc
Description: PGP signature
