Re: [PATCH v6 1/4] file-posix: add tracking of the zone write pointers
On 3/14/23 11:23, Dmitry Fomichev wrote:
>> @@ -3339,10 +3473,27 @@ static int coroutine_fn
>> raw_co_zone_mgmt(BlockDriverState *bs, BlockZoneOp op,
>> len >> BDRV_SECTOR_BITS);
>> ret = raw_thread_pool_submit(bs, handle_aiocb_zone_mgmt, &acb);
>> if (ret != 0) {
>> + update_zones_wp(s->fd, wps, offset, index);
>> ret = -errno;
>> error_report("ioctl %s failed %d", op_name, ret);
>> + goto out;
>> }
>>
>> + if (zo == BLKRESETZONE && len == capacity) {
>> + for (int i = 0; i < bs->bl.nr_zones; ++i) {
>> + if (!BDRV_ZT_IS_CONV(wps->wp[i])) {
>> + wps->wp[i] = i * bs->bl.zone_size;
>
> This will reset write pointers of all read-only zones that may exist on the
> device and make the data stored in those zones unreadable. R/O zones need to
> be
> skipped in this loop.
And offline zones need to be skipped as well.
--
Damien Le Moal
Western Digital Research
Re: [PATCH v6 2/4] block: introduce zone append write for zoned devices
On Fri, 2023-03-10 at 18:31 +0800, Sam Li wrote:
> A zone append command is a write operation that specifies the first
> logical block of a zone as the write position. When writing to a zoned
> block device using zone append, the byte offset of writes is pointing
> to the write pointer of that zone.
s/writes is pointing to the write pointer of that zone/the call may point at any
position within the zone to which the data is being appended/
> Upon completion the device will
> respond with the position the data
s/position the data/position where the data/
> has been written in the zone.
>
> Signed-off-by: Sam Li
With nits above,
Reviewed-by: Dmitry Fomichev
> ---
> block/block-backend.c | 60 +++
> block/file-posix.c | 54 +---
> block/io.c | 21 +++
> block/io_uring.c | 4 +++
> block/linux-aio.c | 3 ++
> block/raw-format.c | 8 +
> include/block/block-io.h | 4 +++
> include/block/block_int-common.h | 5 +++
> include/block/raw-aio.h | 4 ++-
> include/sysemu/block-backend-io.h | 9 +
> 10 files changed, 166 insertions(+), 6 deletions(-)
>
> diff --git a/block/block-backend.c b/block/block-backend.c
> index f70b08e3f6..28e8f5d778 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -1888,6 +1888,45 @@ BlockAIOCB *blk_aio_zone_mgmt(BlockBackend *blk,
> BlockZoneOp op,
> return &acb->common;
> }
>
> +static void coroutine_fn blk_aio_zone_append_entry(void *opaque)
> +{
> + BlkAioEmAIOCB *acb = opaque;
> + BlkRwCo *rwco = &acb->rwco;
> +
> + rwco->ret = blk_co_zone_append(rwco->blk, &acb->bytes,
> + rwco->iobuf, rwco->flags);
> + blk_aio_complete(acb);
> +}
> +
> +BlockAIOCB *blk_aio_zone_append(BlockBackend *blk, int64_t *offset,
> + QEMUIOVector *qiov, BdrvRequestFlags flags,
> + BlockCompletionFunc *cb, void *opaque) {
> + BlkAioEmAIOCB *acb;
> + Coroutine *co;
> + IO_CODE();
> +
> + blk_inc_in_flight(blk);
> + acb = blk_aio_get(&blk_aio_em_aiocb_info, blk, cb, opaque);
> + acb->rwco = (BlkRwCo) {
> + .blk = blk,
> + .ret = NOT_DONE,
> + .flags = flags,
> + .iobuf = qiov,
> + };
> + acb->bytes = *offset;
> + acb->has_returned = false;
> +
> + co = qemu_coroutine_create(blk_aio_zone_append_entry, acb);
> + aio_co_enter(blk_get_aio_context(blk), co);
> + acb->has_returned = true;
> + if (acb->rwco.ret != NOT_DONE) {
> + replay_bh_schedule_oneshot_event(blk_get_aio_context(blk),
> + blk_aio_complete_bh, acb);
> + }
> +
> + return &acb->common;
> +}
> +
> /*
> * Send a zone_report command.
> * offset is a byte offset from the start of the device. No alignment
> @@ -1939,6 +1978,27 @@ int coroutine_fn blk_co_zone_mgmt(BlockBackend *blk,
> BlockZoneOp op,
> return ret;
> }
>
> +/*
> + * Send a zone_append command.
> + */
> +int coroutine_fn blk_co_zone_append(BlockBackend *blk, int64_t *offset,
> + QEMUIOVector *qiov, BdrvRequestFlags flags)
> +{
> + int ret;
> + IO_CODE();
> +
> + blk_inc_in_flight(blk);
> + blk_wait_while_drained(blk);
> + if (!blk_is_available(blk)) {
> + blk_dec_in_flight(blk);
> + return -ENOMEDIUM;
> + }
> +
> + ret = bdrv_co_zone_append(blk_bs(blk), offset, qiov, flags);
> + blk_dec_in_flight(blk);
> + return ret;
> +}
> +
> void blk_drain(BlockBackend *blk)
> {
> BlockDriverState *bs = blk_bs(blk);
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 61ed769ac8..2ba9174778 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -160,6 +160,7 @@ typedef struct BDRVRawState {
> bool has_write_zeroes:1;
> bool use_linux_aio:1;
> bool use_linux_io_uring:1;
> + int64_t *offset; /* offset of zone append operation */
> int page_cache_inconsistent; /* errno from fdatasync failure */
> bool has_fallocate;
> bool needs_alignment;
> @@ -1672,7 +1673,7 @@ static ssize_t handle_aiocb_rw_vector(RawPosixAIOData
> *aiocb)
> ssize_t len;
>
> len = RETRY_ON_EINTR(
> - (aiocb->aio_type & QEMU_AIO_WRITE) ?
> + (aiocb->aio_type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) ?
> qemu_pwritev(aiocb->aio_fildes,
> aiocb->io.iov,
> aiocb->io.niov,
> @@ -1701,7 +1702,7 @@ static ssize_t handle_aiocb_rw_linear(RawPosixAIOData
> *aiocb, char *buf)
> ssize_t len;
>
> while (offset < aiocb->aio_nbytes) {
> - if (aiocb->aio_type & QEMU_AIO_WRITE) {
> + if (aiocb->aio_type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) {
> len = pwrite(aiocb->aio_fildes,
> (const char *)buf + offset,
>
Re: [PATCH v6 4/4] block: add some trace events for zone append
On Fri, 2023-03-10 at 18:31 +0800, Sam Li wrote:
> Signed-off-by: Sam Li
Looks good,
Reviewed-by: Dmitry Fomichev
> block/file-posix.c | 3 +++
> block/trace-events | 2 ++
> 2 files changed, 5 insertions(+)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 2ba9174778..5187f810e5 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -2489,6 +2489,8 @@ out:
> if (!BDRV_ZT_IS_CONV(wps->wp[index])) {
> if (type & QEMU_AIO_ZONE_APPEND) {
> *s->offset = wps->wp[index];
> + trace_zbd_zone_append_complete(bs, *s->offset
> + >> BDRV_SECTOR_BITS);
> }
> /* Advance the wp if needed */
> if (offset + bytes > wps->wp[index]) {
> @@ -3537,6 +3539,7 @@ static int coroutine_fn
> raw_co_zone_append(BlockDriverState *bs,
> len += iov_len;
> }
>
> + trace_zbd_zone_append(bs, *offset >> BDRV_SECTOR_BITS);
> return raw_co_prw(bs, *offset, len, qiov, QEMU_AIO_ZONE_APPEND);
> }
> #endif
> diff --git a/block/trace-events b/block/trace-events
> index 3f4e1d088a..32665158d6 100644
> --- a/block/trace-events
> +++ b/block/trace-events
> @@ -211,6 +211,8 @@ file_hdev_is_sg(int type, int version) "SG device found:
> type=%d, version=%d"
> file_flush_fdatasync_failed(int err) "errno %d"
> zbd_zone_report(void *bs, unsigned int nr_zones, int64_t sector) "bs %p
> report
> %d zones starting at sector offset 0x%" PRIx64 ""
> zbd_zone_mgmt(void *bs, const char *op_name, int64_t sector, int64_t len) "bs
> %p %s starts at sector offset 0x%" PRIx64 " over a range of 0x%" PRIx64 "
> sectors"
> +zbd_zone_append(void *bs, int64_t sector) "bs %p append at sector offset 0x%"
> PRIx64 ""
> +zbd_zone_append_complete(void *bs, int64_t sector) "bs %p returns append
> sector 0x%" PRIx64 ""
>
> # ssh.c
> sftp_error(const char *op, const char *ssh_err, int ssh_err_code, int
> sftp_err_code) "%s failed: %s (libssh error code: %d, sftp error code: %d)"
Re: [PATCH v6 1/4] file-posix: add tracking of the zone write pointers
On Fri, 2023-03-10 at 18:31 +0800, Sam Li wrote:
> Since Linux doesn't have a user API to issue zone append operations to
> zoned devices from user space, the file-posix driver is modified to add
> zone append emulation using regular writes. To do this, the file-posix
> driver tracks the wp location of all zones of the device. It uses an
> array of uint64_t. The most significant bit of each wp location indicates
> if the zone type is conventional zones.
>
> The zones wp can be changed due to the following operations issued:
> - zone reset: change the wp to the start offset of that zone
> - zone finish: change to the end location of that zone
> - write to a zone
> - zone append
>
> Signed-off-by: Sam Li
> ---
> block/file-posix.c | 159 ++-
> include/block/block-common.h | 14 +++
> include/block/block_int-common.h | 3 +
> 3 files changed, 172 insertions(+), 4 deletions(-)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 563acc76ae..61ed769ac8 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -1324,6 +1324,77 @@ static int hdev_get_max_segments(int fd, struct stat
> *st)
> #endif
> }
>
> +#if defined(CONFIG_BLKZONED)
> +static int get_zones_wp(int fd, BlockZoneWps *wps, int64_t offset,
> + unsigned int nrz) {
> + struct blk_zone *blkz;
> + size_t rep_size;
> + uint64_t sector = offset >> BDRV_SECTOR_BITS;
> + int ret, n = 0, i = 0;
> + rep_size = sizeof(struct blk_zone_report) + nrz * sizeof(struct
> blk_zone);
> + g_autofree struct blk_zone_report *rep = NULL;
> +
> + rep = g_malloc(rep_size);
> + blkz = (struct blk_zone *)(rep + 1);
> + while (n < nrz) {
> + memset(rep, 0, rep_size);
> + rep->sector = sector;
> + rep->nr_zones = nrz - n;
> +
> + do {
> + ret = ioctl(fd, BLKREPORTZONE, rep);
> + } while (ret != 0 && errno == EINTR);
> + if (ret != 0) {
> + error_report("%d: ioctl BLKREPORTZONE at %" PRId64 " failed %d",
> + fd, offset, errno);
> + return -errno;
> + }
> +
> + if (!rep->nr_zones) {
> + break;
> + }
> +
> + for (i = 0; i < rep->nr_zones; i++, n++) {
> + /*
> + * The wp tracking cares only about sequential writes required
> and
> + * sequential write preferred zones so that the wp can advance to
> + * the right location.
> + * Use the most significant bit of the wp location to indicate
> the
> + * zone type: 0 for SWR/SWP zones and 1 for conventional zones.
> + */
> + if (blkz[i].type == BLK_ZONE_TYPE_CONVENTIONAL) {
> + wps->wp[i] = 1ULL << 63;
> + } else {
> + switch(blkz[i].cond) {
> + case BLK_ZONE_COND_FULL:
> + case BLK_ZONE_COND_READONLY:
> + /* Zone not writable */
> + wps->wp[i] = (blkz[i].start + blkz[i].len) <<
> BDRV_SECTOR_BITS;
> + break;
> + case BLK_ZONE_COND_OFFLINE:
> + /* Zone not writable nor readable */
> + wps->wp[i] = (blkz[i].start) << BDRV_SECTOR_BITS;
> + break;
> + default:
> + wps->wp[i] = blkz[i].wp << BDRV_SECTOR_BITS;
> + break;
> + }
> + }
> + }
> + sector = blkz[i - 1].start + blkz[i - 1].len;
> + }
> +
> + return 0;
> +}
> +
> +static void update_zones_wp(int fd, BlockZoneWps *wps, int64_t offset,
> + unsigned int nrz) {
> + if (get_zones_wp(fd, wps, offset, nrz) < 0) {
> + error_report("update zone wp failed");
> + }
> +}
> +#endif
> +
> static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
> {
> BDRVRawState *s = bs->opaque;
> @@ -1413,6 +1484,21 @@ static void raw_refresh_limits(BlockDriverState *bs,
> Error **errp)
> if (ret >= 0) {
> bs->bl.max_active_zones = ret;
> }
> +
> + ret = get_sysfs_long_val(&st, "physical_block_size");
> + if (ret >= 0) {
> + bs->bl.write_granularity = ret;
> + }
> +
> + bs->bl.wps = g_malloc(sizeof(BlockZoneWps) +
> + sizeof(int64_t) * bs->bl.nr_zones);
> + ret = get_zones_wp(s->fd, bs->bl.wps, 0, bs->bl.nr_zones);
> + if (ret < 0) {
> + error_setg_errno(errp, -ret, "report wps failed");
> + g_free(bs->bl.wps);
> + return;
> + }
> + qemu_co_mutex_init(&bs->bl.wps->colock);
> return;
> }
> out:
> @@ -2338,9 +2424,15 @@ static int coroutine_fn raw_co_prw(BlockDriverState
> *bs,
> uint64_t offset,
> {
> BDRVRawState *s = bs->opaque;
> RawPosixAIOData acb;
> + int ret;
>
> if (fd_open(bs) < 0)
> return -EIO;
> +#if defined(CON
Re: [PATCH v16 7/8] block: add some trace events for new block layer APIs
On Fri, 2023-03-10 at 18:24 +0800, Sam Li wrote:
> Signed-off-by: Sam Li
> Reviewed-by: Stefan Hajnoczi
With one small nit below,
Reviewed-by: Dmitry Fomichev
> ---
> block/file-posix.c | 3 +++
> block/trace-events | 2 ++
> 2 files changed, 5 insertions(+)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 2eceb250f1..563acc76ae 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -3256,6 +3256,7 @@ static int coroutine_fn
> raw_co_zone_report(BlockDriverState *bs, int64_t offset,
> BlockZoneDescriptor *zones) {
> BDRVRawState *s = bs->opaque;
> RawPosixAIOData acb;
> + trace_zbd_zone_report(bs, *nr_zones, offset >> BDRV_SECTOR_BITS);
The code in this function could be made a bit simpler -
BDRVRawState *s = bs->opaque;
RawPosixAIOData acb = (RawPosixAIOData) {
.bs = bs,
.aio_fildes = s->fd,
.aio_type = QEMU_AIO_ZONE_REPORT,
.aio_offset = offset,
.zone_report= {
.nr_zones = nr_zones,
.zones = zones,
},
};
trace_zbd_zone_report(bs, *nr_zones, offset >> BDRV_SECTOR_BITS);
return raw_thread_pool_submit(bs, handle_aiocb_zone_report, &acb);
>
> acb = (RawPosixAIOData) {
> .bs = bs,
> @@ -3334,6 +3335,8 @@ static int coroutine_fn
> raw_co_zone_mgmt(BlockDriverState
> *bs, BlockZoneOp op,
> },
> };
>
> + trace_zbd_zone_mgmt(bs, op_name, offset >> BDRV_SECTOR_BITS,
> + len >> BDRV_SECTOR_BITS);
> ret = raw_thread_pool_submit(bs, handle_aiocb_zone_mgmt, &acb);
> if (ret != 0) {
> ret = -errno;
> diff --git a/block/trace-events b/block/trace-events
> index 48dbf10c66..3f4e1d088a 100644
> --- a/block/trace-events
> +++ b/block/trace-events
> @@ -209,6 +209,8 @@ file_FindEjectableOpticalMedia(const char *media)
> "Matching
> using %s"
> file_setup_cdrom(const char *partition) "Using %s as optical disc"
> file_hdev_is_sg(int type, int version) "SG device found: type=%d, version=%d"
> file_flush_fdatasync_failed(int err) "errno %d"
> +zbd_zone_report(void *bs, unsigned int nr_zones, int64_t sector) "bs %p
> report
> %d zones starting at sector offset 0x%" PRIx64 ""
> +zbd_zone_mgmt(void *bs, const char *op_name, int64_t sector, int64_t len) "bs
> %p %s starts at sector offset 0x%" PRIx64 " over a range of 0x%" PRIx64 "
> sectors"
>
> # ssh.c
> sftp_error(const char *op, const char *ssh_err, int ssh_err_code, int
> sftp_err_code) "%s failed: %s (libssh error code: %d, sftp error code: %d)"
Re: [PATCH v16 3/8] block: add block layer APIs resembling Linux ZonedBlockDevice ioctls
On Fri, 2023-03-10 at 18:23 +0800, Sam Li wrote:
> Add zoned device option to host_device BlockDriver. It will be presented only
> for zoned host block devices. By adding zone management operations to the
> host_block_device BlockDriver, users can use the new block layer APIs
> including Report Zone and four zone management operations
> (open, close, finish, reset, reset_all).
>
> Qemu-io uses the new APIs to perform zoned storage commands of the device:
> zone_report(zrp), zone_open(zo), zone_close(zc), zone_reset(zrs),
> zone_finish(zf).
>
> For example, to test zone_report, use following command:
> $ ./build/qemu-io --image-opts -n driver=host_device, filename=/dev/nullb0
> -c "zrp offset nr_zones"
>
> Signed-off-by: Sam Li
> Reviewed-by: Hannes Reinecke
> Reviewed-by: Stefan Hajnoczi
LGTM,
Reviewed-by: Dmitry Fomichev
> Acked-by: Kevin Wolf
> ---
> block/block-backend.c | 133 +
> block/file-posix.c | 309 +-
> block/io.c | 41
> include/block/block-io.h | 9 +
> include/block/block_int-common.h | 21 ++
> include/block/raw-aio.h | 6 +-
> include/sysemu/block-backend-io.h | 18 ++
> meson.build | 4 +
> qemu-io-cmds.c | 149 ++
> 9 files changed, 687 insertions(+), 3 deletions(-)
>
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 278b04ce69..f70b08e3f6 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -1806,6 +1806,139 @@ int coroutine_fn blk_co_flush(BlockBackend *blk)
> return ret;
> }
>
> +static void coroutine_fn blk_aio_zone_report_entry(void *opaque)
> +{
> + BlkAioEmAIOCB *acb = opaque;
> + BlkRwCo *rwco = &acb->rwco;
> +
> + rwco->ret = blk_co_zone_report(rwco->blk, rwco->offset,
> + (unsigned int*)acb->bytes,rwco->iobuf);
> + blk_aio_complete(acb);
> +}
> +
> +BlockAIOCB *blk_aio_zone_report(BlockBackend *blk, int64_t offset,
> + unsigned int *nr_zones,
> + BlockZoneDescriptor *zones,
> + BlockCompletionFunc *cb, void *opaque)
> +{
> + BlkAioEmAIOCB *acb;
> + Coroutine *co;
> + IO_CODE();
> +
> + blk_inc_in_flight(blk);
> + acb = blk_aio_get(&blk_aio_em_aiocb_info, blk, cb, opaque);
> + acb->rwco = (BlkRwCo) {
> + .blk = blk,
> + .offset = offset,
> + .iobuf = zones,
> + .ret = NOT_DONE,
> + };
> + acb->bytes = (int64_t)nr_zones,
> + acb->has_returned = false;
> +
> + co = qemu_coroutine_create(blk_aio_zone_report_entry, acb);
> + aio_co_enter(blk_get_aio_context(blk), co);
> +
> + acb->has_returned = true;
> + if (acb->rwco.ret != NOT_DONE) {
> + replay_bh_schedule_oneshot_event(blk_get_aio_context(blk),
> + blk_aio_complete_bh, acb);
> + }
> +
> + return &acb->common;
> +}
> +
> +static void coroutine_fn blk_aio_zone_mgmt_entry(void *opaque)
> +{
> + BlkAioEmAIOCB *acb = opaque;
> + BlkRwCo *rwco = &acb->rwco;
> +
> + rwco->ret = blk_co_zone_mgmt(rwco->blk, (BlockZoneOp)rwco->iobuf,
> + rwco->offset, acb->bytes);
> + blk_aio_complete(acb);
> +}
> +
> +BlockAIOCB *blk_aio_zone_mgmt(BlockBackend *blk, BlockZoneOp op,
> + int64_t offset, int64_t len,
> + BlockCompletionFunc *cb, void *opaque) {
> + BlkAioEmAIOCB *acb;
> + Coroutine *co;
> + IO_CODE();
> +
> + blk_inc_in_flight(blk);
> + acb = blk_aio_get(&blk_aio_em_aiocb_info, blk, cb, opaque);
> + acb->rwco = (BlkRwCo) {
> + .blk = blk,
> + .offset = offset,
> + .iobuf = (void *)op,
> + .ret = NOT_DONE,
> + };
> + acb->bytes = len;
> + acb->has_returned = false;
> +
> + co = qemu_coroutine_create(blk_aio_zone_mgmt_entry, acb);
> + aio_co_enter(blk_get_aio_context(blk), co);
> +
> + acb->has_returned = true;
> + if (acb->rwco.ret != NOT_DONE) {
> + replay_bh_schedule_oneshot_event(blk_get_aio_context(blk),
> + blk_aio_complete_bh, acb);
> + }
> +
> + return &acb->common;
> +}
> +
> +/*
> + * Send a zone_report command.
> + * offset is a byte offset from the start of the device. No alignment
> + * required for offset.
> + * nr_zones represents IN maximum and OUT actual.
> + */
> +int coroutine_fn blk_co_zone_report(BlockBackend *blk, int64_t offset,
> + unsigned int *nr_zones,
> + BlockZoneDescriptor *zones)
> +{
> + int ret;
> + IO_CODE();
> +
> + blk_inc_in_flight(blk); /* increase before waiting */
> + blk_wait_while_drained(blk);
> + if (!blk_is_available(blk)) {
> + blk_dec_in_flight(blk);
> + return -ENOMEDIUM;
> +
Re: [PATCH v16 1/8] include: add zoned device structs
On Fri, 2023-03-10 at 18:23 +0800, Sam Li wrote:
> Signed-off-by: Sam Li
> Reviewed-by: Stefan Hajnoczi
> Reviewed-by: Damien Le Moal
> Reviewed-by: Hannes Reinecke
Looks good to me.
Reviewed-by: Dmitry Fomichev
> ---
> include/block/block-common.h | 43
> 1 file changed, 43 insertions(+)
>
> diff --git a/include/block/block-common.h b/include/block/block-common.h
> index b5122ef8ab..1576fcf2ed 100644
> --- a/include/block/block-common.h
> +++ b/include/block/block-common.h
> @@ -75,6 +75,49 @@ typedef struct BlockDriver BlockDriver;
> typedef struct BdrvChild BdrvChild;
> typedef struct BdrvChildClass BdrvChildClass;
>
> +typedef enum BlockZoneOp {
> + BLK_ZO_OPEN,
> + BLK_ZO_CLOSE,
> + BLK_ZO_FINISH,
> + BLK_ZO_RESET,
> +} BlockZoneOp;
> +
> +typedef enum BlockZoneModel {
> + BLK_Z_NONE = 0x0, /* Regular block device */
> + BLK_Z_HM = 0x1, /* Host-managed zoned block device */
> + BLK_Z_HA = 0x2, /* Host-aware zoned block device */
> +} BlockZoneModel;
> +
> +typedef enum BlockZoneState {
> + BLK_ZS_NOT_WP = 0x0,
> + BLK_ZS_EMPTY = 0x1,
> + BLK_ZS_IOPEN = 0x2,
> + BLK_ZS_EOPEN = 0x3,
> + BLK_ZS_CLOSED = 0x4,
> + BLK_ZS_RDONLY = 0xD,
> + BLK_ZS_FULL = 0xE,
> + BLK_ZS_OFFLINE = 0xF,
> +} BlockZoneState;
> +
> +typedef enum BlockZoneType {
> + BLK_ZT_CONV = 0x1, /* Conventional random writes supported */
> + BLK_ZT_SWR = 0x2, /* Sequential writes required */
> + BLK_ZT_SWP = 0x3, /* Sequential writes preferred */
> +} BlockZoneType;
> +
> +/*
> + * Zone descriptor data structure.
> + * Provides information on a zone with all position and size values in bytes.
> + */
> +typedef struct BlockZoneDescriptor {
> + uint64_t start;
> + uint64_t length;
> + uint64_t cap;
> + uint64_t wp;
> + BlockZoneType type;
> + BlockZoneState state;
> +} BlockZoneDescriptor;
> +
> typedef struct BlockDriverInfo {
> /* in bytes, 0 if irrelevant */
> int cluster_size;
Re: [PATCH] include/blcok: fixup typos
On Mon, 2023-03-13 at 10:01 +, Peter Maydell wrote: > On Mon, 13 Mar 2023 at 00:26, Wilfred Mallawa > wrote: > > > > From: Wilfred Mallawa > > > > Fixup a few minor typos > > Typo in patch subject line: should be 'block' :-) Ha! already sent a V2 for this :) > > > Signed-off-by: Wilfred Mallawa > > --- > > Otherwise > Reviewed-by: Peter Maydell > > thanks > -- PMM
Re: [PULL v2 00/25] Win socket patches
On Mon, 13 Mar 2023 at 11:46, wrote: > > From: Marc-André Lureau > > The following changes since commit 29c8a9e31a982874ce4e2c15f2bf82d5f8dc3517: > > Merge tag 'linux-user-for-8.0-pull-request' of > https://gitlab.com/laurent_vivier/qemu into staging (2023-03-12 10:57:00 > +) > > are available in the Git repository at: > > https://gitlab.com/marcandre.lureau/qemu.git tags/win-socket-pull-request > > for you to fetch changes up to 4bf21c7f748bee42b6f4692f8c37a11d1033b2d1: > > monitor: restrict command getfd to POSIX hosts (2023-03-13 15:46:09 +0400) > > > QMP command to import win32 sockets > > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/8.0 for any user-visible changes. -- PMM
Re: [PATCH for-8.0] ide: Fix manual in-flight count for TRIM BH
Am 10.03.2023 um 16:13 hat Paolo Bonzini geschrieben: > On Fri, Mar 10, 2023 at 3:25 PM Kevin Wolf wrote: > > > 1. The TRIM operation should be completed on the IDE level before > > > draining ends. > > > 2. Block layer requests issued after draining has begun are queued. > > > > > > To me, the conclusion seems to be: > > > Issue all block layer requests belonging to the IDE TRIM operation up > > > front. > > > > > > The other alternative I see is to break assumption 2, introduce a way > > > to not queue certain requests while drained, and use it for the > > > recursive requests issued by ide_issue_trim_cb. But not the initial > > > one, if that would defeat the purpose of request queuing. Of course > > > this can't be done if QEMU relies on the assumption in other places > > > already. > > > > I feel like this should be allowed because if anyone has exclusive > > access in this scenario, it's IDE, so it should be able to bypass the > > queuing. Of course, the queuing is still needed if someone else drained > > the backend, so we can't just make TRIM bypass it in general. And if you > > make it conditional on IDE being in blk_drain(), it already starts to > > become ugly again... > > > > So maybe the while loop is unavoidable. > > > > Hmm... But could ide_cancel_dma_sync() just directly use > > AIO_WAIT_WHILE(s->bus->dma->aiocb) instead of using blk_drain()? > > While that should work, it would not fix other uses of > bdrv_drain_all(), for example in softmmu/cpus.c. Stopping the device > model relies on those to run *until the device model has finished > submitting requests*. If so, do_vm_stop() really expects drain to do something it isn't designed to do. It's only for quiescing backends, not for any other activity a qdev device might still be doing. I think it's really the vm_state_notify() that should take care of stopping device activity. But maybe we can make it work with drain anyway. > So I still think that this bug is a symptom of a problem in the design > of request queuing. > > In fact, shouldn't request queuing was enabled at the _end_ of > bdrv_drained_begin (once the BlockBackend has reached a quiescent > state on its own terms), rather than at the beginning (which leads to > deadlocks like this one)? No, I don't think that is ever right. As I said earlier in this thread (and you said yourself previously), there are two different users of drain: 1. I want to have exclusive access to the node. This one wants request queuing from the start to avoid losing time unnecessarily until the guest stops sending new requests. 2. I want to wait for my requests to complete. This one never wants request queuing. Enabling it at the end of bdrv_drained_begin() wouldn't hurt it (because it has already achieved its goal then), but it's also not necessary for the same reason. IDE reset and do_vm_stop() are case 2, implemented with blk_drain*(). The request queuing was implemented for case 1, something else in the block graph draining the BlockBackend's root node with bdrv_drain*(). So maybe what we could take from this is that request queuing should be temporarily disabled while we're in blk_drain*() because these interfaces are only meant for case 2. In all other cases, it should continue to work as it does now. Kevin
Re: [PATCH for-8.0] ide: Fix manual in-flight count for TRIM BH
On 3/13/23 13:29, Fiona Ebner wrote: In fact, shouldn't request queuing was enabled at the _end_ of bdrv_drained_begin (once the BlockBackend has reached a quiescent state on its own terms), rather than at the beginning (which leads to deadlocks like this one)? Couldn't this lead to scenarios where a busy or malicious guest, which continues to submit new requests, slows down draining or even prevents it from finishing? Possibly, but there is also a .drained_begin/.drained_end callback that can be defined in order to apply backpressure. (For some other devices, there's also aio_disable_external/aio_enable_external that do the equivalent of request queuing but without the deadlocks) Since starting the queuing of requests at the end of bdrv_drained_begin wouldn't hurt correctness, and it would fix this kind of deadlock, I think it would be worth giving it a try. Paolo
Re: [PATCH for-8.0] ide: Fix manual in-flight count for TRIM BH
Am 10.03.23 um 16:13 schrieb Paolo Bonzini:
> On Fri, Mar 10, 2023 at 3:25 PM Kevin Wolf wrote:
>>> 1. The TRIM operation should be completed on the IDE level before
>>> draining ends.
>>> 2. Block layer requests issued after draining has begun are queued.
>>>
>>> To me, the conclusion seems to be:
>>> Issue all block layer requests belonging to the IDE TRIM operation up
>>> front.
>>>
>>> The other alternative I see is to break assumption 2, introduce a way
>>> to not queue certain requests while drained, and use it for the
>>> recursive requests issued by ide_issue_trim_cb. But not the initial
>>> one, if that would defeat the purpose of request queuing. Of course
>>> this can't be done if QEMU relies on the assumption in other places
>>> already.
>>
>> I feel like this should be allowed because if anyone has exclusive
>> access in this scenario, it's IDE, so it should be able to bypass the
>> queuing. Of course, the queuing is still needed if someone else drained
>> the backend, so we can't just make TRIM bypass it in general. And if you
>> make it conditional on IDE being in blk_drain(), it already starts to
>> become ugly again...
>>
>> So maybe the while loop is unavoidable.
>>
>> Hmm... But could ide_cancel_dma_sync() just directly use
>> AIO_WAIT_WHILE(s->bus->dma->aiocb) instead of using blk_drain()?
>
> While that should work, it would not fix other uses of
> bdrv_drain_all(), for example in softmmu/cpus.c. Stopping the device
> model relies on those to run *until the device model has finished
> submitting requests*.
>
> So I still think that this bug is a symptom of a problem in the design
> of request queuing.
>
> In fact, shouldn't request queuing was enabled at the _end_ of
> bdrv_drained_begin (once the BlockBackend has reached a quiescent
> state on its own terms), rather than at the beginning (which leads to
> deadlocks like this one)?
>
> blk->quiesce_counter becomes just a nesting counter for
> drained_begin/end, with no uses outside, and blk_wait_while_drained
> uses a new counter. Then you have something like this in
> blk_root_drained_poll:
>
> if (blk->dev_ops && blk->dev_ops->drained_poll) {
> busy = blk->dev_ops->drained_poll(blk->dev_opaque);
> }
> busy |= !!blk->in_flight;
> if (!busy) {
>qatomic_set(&blk->queue_requests, true);
> }
> return busy;
>
> And there's no need to touch IDE at all.
>
Couldn't this lead to scenarios where a busy or malicious guest, which
continues to submit new requests, slows down draining or even prevents
it from finishing?
Best Regards,
Fiona
Qemu-storage-daemon crash with 8ab8140 or later
Hello folks, I found a functional regression in qemu which is making qemu-storage-daemon to crash when using /dev/ram0 as the host device. I bisected it up to: commit 8ab8140a04cf771d63e9754d6ba6c1e676bfe507 Author: Kevin Wolf Date: Fri Feb 3 16:22:02 2023 +0100 block: Mark bdrv_co_refresh_total_sectors() and callers GRAPH_RDLOCK This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_co_refresh_total_sectors() need to hold a reader lock for the graph. Signed-off-by: Kevin Wolf Message-Id: <20230203152202.49054-24-kw...@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito Signed-off-by: Kevin Wolf To reproduce it create the export by: mkdir -p /var/lib/runperf/runperf-libblkio/ modprobe brd rd_nr=1 rd_size=1048576 max_part=0 qemu-storage-daemon --blockdev driver=host_device,node-name=file,filename=/dev/ram0,cache.direct=on --object iothread,id=iothread0 --export type=vhost-user-blk,iothread=iothread0,id=export,node-name=file,addr.type=unix,addr.path=/var/lib/runperf/runperf-libblkio/vhost-user-blk.sock,writable=on Then create the following fio job in /var/lib/runperf/runperf-libblkio/libblkio.fio: [global] bs=4k ioengine=libblkio libblkio_driver=virtio-blk-vhost-user libblkio_path=/var/lib/runperf/runperf-libblkio/vhost-user-blk.sock rw=read iodepth=1 hipri=1# Can not be set by pbench-fio, use 0 or 1 direct=1 sync=0 time_based=1 clocksource=gettimeofday ramp_time=5 runtime=10 write_bw_log=fio write_iops_log=fio write_lat_log=fio log_avg_msec=1000 write_hist_log=fio log_hist_msec=1 # log_hist_coarseness=4 # 76 bins [job0] And run it using: timeout -s 9 20 fio /var/lib/runperf/runperf-libblkio/libblkio.fio Before the 8ab8140a04cf771d63e9754d6ba6c1e676bfe507 this worked well, using every newer commit results in qemu-storage-daemon coredump and fio hang (which is why I added the timeout to this example) Coredump: [root@virtlab722 ~]# coredumpctl info PID: 40972 (qemu-storage-da) UID: 0 (root) GID: 0 (root) Signal: 6 (ABRT) Timestamp: Mon 2023-03-13 07:51:09 EDT (34s ago) Command Line: qemu-storage-daemon --blockdev driver=host_device,node-name=file,filename=/dev/ram0,cache.direct=on --object iothread,id=iothread0 -> Executable: /usr/local/bin/qemu-storage-daemon Control Group: /user.slice/user-0.slice/session-5.scope Unit: session-5.scope Slice: user-0.slice Session: 5 Owner UID: 0 (root) Boot ID: 7705815f01cb4ccefe6ccfe2c0ba Machine ID: 617b70389a7d4b4280ec97bdcb203def Hostname: virtlab722.virt.lab.eng.bos.redhat.com Storage: /var/lib/systemd/coredump/core.qemu-storage-da.0.7705815f01cb4ccefe6ccfe2c0ba.40972.167870826900.zst (present) Size on Disk: 115.8K Message: Process 40972 (qemu-storage-da) of user 0 dumped core. Stack trace of thread 40974: #0 0x7f89c22a154c __pthread_kill_implementation (libc.so.6 + 0xa154c) #1 0x7f89c2254d46 raise (libc.so.6 + 0x54d46) #2 0x7f89c22287f3 abort (libc.so.6 + 0x287f3) #3 0x7f89c222871b __assert_fail_base.cold (libc.so.6 + 0x2871b) #4 0x7f89c224dce6 __assert_fail (libc.so.6 + 0x4dce6) #5 0x5577565ac060 assert_bdrv_graph_readable (qemu-storage-daemon + 0xb9060) #6 0x557756585552 bdrv_co_nb_sectors (qemu-storage-daemon + 0x92552) #7 0x557756585639 bdrv_get_geometry (qemu-storage-daemon + 0x92639) #8 0x557756570a55 virtio_blk_sect_range_ok (qemu-storage-daemon + 0x7da55) #9 0x557756570387 vu_blk_virtio_process_req (qemu-storage-daemon + 0x7d387) #10 0x55775669cbcb coroutine_trampoline (qemu-storage-daemon + 0x1a9bcb) #11 0x7f89c222a360 n/a (libc.so.6 + 0x2a360) ELF object binary architecture: AMD x86-64 Regards, Lukáš Doktor OpenPGP_0x26B362E47FCF22C1.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
[PULL 18/25] tests/docker: fix a win32 error due to portability
From: Marc-André Lureau
docker.py is run during configure, and produces an error: No module
named 'pwd'.
Use a more portable and recommended alternative to lookup the user
"login name".
Signed-off-by: Marc-André Lureau
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <20230306122751.2355515-4-marcandre.lur...@redhat.com>
---
tests/docker/docker.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 3a1ed7cb18..688ef62989 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -23,10 +23,10 @@
import tempfile
import re
import signal
+import getpass
from tarfile import TarFile, TarInfo
from io import StringIO, BytesIO
from shutil import copy, rmtree
-from pwd import getpwuid
from datetime import datetime, timedelta
@@ -316,7 +316,7 @@ def build_image(self, tag, docker_dir, dockerfile,
if user:
uid = os.getuid()
-uname = getpwuid(uid).pw_name
+uname = getpass.getuser()
tmp_df.write("\n")
tmp_df.write("RUN id %s 2>/dev/null || useradd -u %d -U %s" %
(uname, uid, uname))
@@ -570,7 +570,7 @@ def run(self, args, argv):
if args.user:
uid = os.getuid()
-uname = getpwuid(uid).pw_name
+uname = getpass.getuser()
df.write("\n")
df.write("RUN id %s 2>/dev/null || useradd -u %d -U %s" %
(uname, uid, uname))
--
2.39.2
[PULL v2 25/25] monitor: restrict command getfd to POSIX hosts
From: Marc-André Lureau
Currently, the function will simply fail if ancillary fds are not
provided, for ex on unsupported platforms.
This changes the failure from:
{"error": {"class": "GenericError", "desc": "No file descriptor
supplied via SCM_RIGHTS"}}
to:
{"error": {"class": "CommandNotFound", "desc": "The command getfd
has not been found"}}
Signed-off-by: Marc-André Lureau
Reviewed-by: Markus Armbruster
---
qapi/misc.json | 2 +-
monitor/fds.c | 2 ++
monitor/hmp-cmds.c | 2 ++
hmp-commands.hx| 2 ++
4 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/qapi/misc.json b/qapi/misc.json
index 5ef6286af3..6ddd16ea28 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -273,7 +273,7 @@
# <- { "return": {} }
#
##
-{ 'command': 'getfd', 'data': {'fdname': 'str'} }
+{ 'command': 'getfd', 'data': {'fdname': 'str'}, 'if': 'CONFIG_POSIX' }
##
# @get-win32-socket:
diff --git a/monitor/fds.c b/monitor/fds.c
index 9ed4197358..d86c2c674c 100644
--- a/monitor/fds.c
+++ b/monitor/fds.c
@@ -98,6 +98,7 @@ static bool monitor_add_fd(Monitor *mon, int fd, const char
*fdname, Error **err
return true;
}
+#ifdef CONFIG_POSIX
void qmp_getfd(const char *fdname, Error **errp)
{
Monitor *cur_mon = monitor_cur();
@@ -111,6 +112,7 @@ void qmp_getfd(const char *fdname, Error **errp)
monitor_add_fd(cur_mon, fd, fdname, errp);
}
+#endif
void qmp_closefd(const char *fdname, Error **errp)
{
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 34bd8c67d7..6c559b48c8 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -192,6 +192,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, err);
}
+#ifdef CONFIG_POSIX
void hmp_getfd(Monitor *mon, const QDict *qdict)
{
const char *fdname = qdict_get_str(qdict, "fdname");
@@ -200,6 +201,7 @@ void hmp_getfd(Monitor *mon, const QDict *qdict)
qmp_getfd(fdname, &err);
hmp_handle_error(mon, err);
}
+#endif
void hmp_closefd(Monitor *mon, const QDict *qdict)
{
diff --git a/hmp-commands.hx b/hmp-commands.hx
index b87c250e23..bb85ee1d26 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1486,6 +1486,7 @@ SRST
Inject an MCE on the given CPU (x86 only).
ERST
+#ifdef CONFIG_POSIX
{
.name = "getfd",
.args_type = "fdname:s",
@@ -1501,6 +1502,7 @@ SRST
mechanism on unix sockets, it is stored using the name *fdname* for
later use by other monitor commands.
ERST
+#endif
{
.name = "closefd",
--
2.39.2
[PULL v2 07/25] win32/socket: introduce qemu_socket_unselect() helper
From: Marc-André Lureau
A more explicit version of qemu_socket_select() with no events.
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-8-marcandre.lur...@redhat.com>
---
include/sysemu/os-win32.h | 2 ++
io/channel-socket.c | 4 ++--
util/oslib-win32.c| 7 ++-
3 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 9f842ae643..504a8966c3 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -169,6 +169,8 @@ static inline void qemu_funlockfile(FILE *f)
bool qemu_socket_select(SOCKET s, WSAEVENT hEventObject,
long lNetworkEvents, Error **errp);
+bool qemu_socket_unselect(SOCKET s, Error **errp);
+
/* We wrap all the sockets functions so that we can
* set errno based on WSAGetLastError()
*/
diff --git a/io/channel-socket.c b/io/channel-socket.c
index 0bc29c4808..03757c7a7e 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -442,7 +442,7 @@ static void qio_channel_socket_finalize(Object *obj)
}
}
#ifdef WIN32
-qemu_socket_select(ioc->fd, NULL, 0, NULL);
+qemu_socket_unselect(ioc->fd, NULL);
#endif
closesocket(ioc->fd);
ioc->fd = -1;
@@ -846,7 +846,7 @@ qio_channel_socket_close(QIOChannel *ioc,
if (sioc->fd != -1) {
#ifdef WIN32
-qemu_socket_select(sioc->fd, NULL, 0, NULL);
+qemu_socket_unselect(sioc->fd, NULL);
#endif
if (qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_LISTEN)) {
socket_listen_cleanup(sioc->fd, errp);
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index df752fc762..dbd32acc98 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -180,7 +180,7 @@ static int socket_error(void)
void qemu_socket_set_block(int fd)
{
unsigned long opt = 0;
-qemu_socket_select(fd, NULL, 0, NULL);
+qemu_socket_unselect(fd, NULL);
ioctlsocket(fd, FIONBIO, &opt);
}
@@ -298,6 +298,11 @@ bool qemu_socket_select(SOCKET s, WSAEVENT hEventObject,
return true;
}
+bool qemu_socket_unselect(SOCKET s, Error **errp)
+{
+return qemu_socket_select(s, NULL, 0, errp);
+}
+
#undef connect
int qemu_connect_wrap(int sockfd, const struct sockaddr *addr,
socklen_t addrlen)
--
2.39.2
[PULL 25/25] QMP/HMP: only actually implement getfd on CONFIG_POSIX
From: Marc-André Lureau
Currently, the function will simply fail if ancillary fds are not
provided, for ex on unsupported platforms.
Signed-off-by: Marc-André Lureau
Reviewed-by: Markus Armbruster
Message-Id: <20230306122751.2355515-12-marcandre.lur...@redhat.com>
---
qapi/misc.json | 2 +-
monitor/fds.c | 2 ++
monitor/hmp-cmds.c | 2 ++
hmp-commands.hx| 2 ++
4 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/qapi/misc.json b/qapi/misc.json
index 5ef6286af3..6ddd16ea28 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -273,7 +273,7 @@
# <- { "return": {} }
#
##
-{ 'command': 'getfd', 'data': {'fdname': 'str'} }
+{ 'command': 'getfd', 'data': {'fdname': 'str'}, 'if': 'CONFIG_POSIX' }
##
# @get-win32-socket:
diff --git a/monitor/fds.c b/monitor/fds.c
index 9ed4197358..d86c2c674c 100644
--- a/monitor/fds.c
+++ b/monitor/fds.c
@@ -98,6 +98,7 @@ static bool monitor_add_fd(Monitor *mon, int fd, const char
*fdname, Error **err
return true;
}
+#ifdef CONFIG_POSIX
void qmp_getfd(const char *fdname, Error **errp)
{
Monitor *cur_mon = monitor_cur();
@@ -111,6 +112,7 @@ void qmp_getfd(const char *fdname, Error **errp)
monitor_add_fd(cur_mon, fd, fdname, errp);
}
+#endif
void qmp_closefd(const char *fdname, Error **errp)
{
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 34bd8c67d7..6c559b48c8 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -192,6 +192,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, err);
}
+#ifdef CONFIG_POSIX
void hmp_getfd(Monitor *mon, const QDict *qdict)
{
const char *fdname = qdict_get_str(qdict, "fdname");
@@ -200,6 +201,7 @@ void hmp_getfd(Monitor *mon, const QDict *qdict)
qmp_getfd(fdname, &err);
hmp_handle_error(mon, err);
}
+#endif
void hmp_closefd(Monitor *mon, const QDict *qdict)
{
diff --git a/hmp-commands.hx b/hmp-commands.hx
index b87c250e23..bb85ee1d26 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1486,6 +1486,7 @@ SRST
Inject an MCE on the given CPU (x86 only).
ERST
+#ifdef CONFIG_POSIX
{
.name = "getfd",
.args_type = "fdname:s",
@@ -1501,6 +1502,7 @@ SRST
mechanism on unix sockets, it is stored using the name *fdname* for
later use by other monitor commands.
ERST
+#endif
{
.name = "closefd",
--
2.39.2
[PULL v2 17/25] char: do not double-close fd when failing to add client
From: Marc-André Lureau
The caller is already closing the fd on failure.
Fixes: c3054a6e6a ("char: Factor out qmp_add_client() parts and move to
chardev/")
Signed-off-by: Marc-André Lureau
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Thomas Huth
Message-Id: <20230306122751.2355515-3-marcandre.lur...@redhat.com>
---
chardev/char.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/chardev/char.c b/chardev/char.c
index 11eab7764c..e69390601f 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -1175,12 +1175,10 @@ bool qmp_add_client_char(int fd, bool has_skipauth,
bool skipauth,
if (!s) {
error_setg(errp, "protocol '%s' is invalid", protocol);
-close(fd);
return false;
}
if (qemu_chr_add_client(s, fd) < 0) {
error_setg(errp, "failed to add client");
-close(fd);
return false;
}
return true;
--
2.39.2
[PULL 17/25] char: do not double-close fd when failing to add client
From: Marc-André Lureau
The caller is already closing the fd on failure.
Fixes: c3054a6e6a ("char: Factor out qmp_add_client() parts and move to
chardev/")
Signed-off-by: Marc-André Lureau
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Thomas Huth
Message-Id: <20230306122751.2355515-3-marcandre.lur...@redhat.com>
---
chardev/char.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/chardev/char.c b/chardev/char.c
index 11eab7764c..e69390601f 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -1175,12 +1175,10 @@ bool qmp_add_client_char(int fd, bool has_skipauth,
bool skipauth,
if (!s) {
error_setg(errp, "protocol '%s' is invalid", protocol);
-close(fd);
return false;
}
if (qemu_chr_add_client(s, fd) < 0) {
error_setg(errp, "failed to add client");
-close(fd);
return false;
}
return true;
--
2.39.2
[PULL v2 19/25] osdep: implement qemu_socketpair() for win32
From: Marc-André Lureau
Manually implement a socketpair() function, using UNIX sockets and
simple peer credential checking.
QEMU doesn't make much use of socketpair, beside vhost-user which is not
available for win32 at this point. However, I intend to use it for
writing some new portable tests.
Signed-off-by: Marc-André Lureau
Reviewed-by: Daniel P. Berrangé
Message-Id: <20230306122751.2355515-5-marcandre.lur...@redhat.com>
---
include/qemu/sockets.h | 2 -
util/oslib-win32.c | 110 +
2 files changed, 110 insertions(+), 2 deletions(-)
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 2b0698a7c9..d935fd80da 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -15,7 +15,6 @@ int inet_aton(const char *cp, struct in_addr *ia);
bool fd_is_socket(int fd);
int qemu_socket(int domain, int type, int protocol);
-#ifndef WIN32
/**
* qemu_socketpair:
* @domain: specifies a communication domain, such as PF_UNIX
@@ -30,7 +29,6 @@ int qemu_socket(int domain, int type, int protocol);
* Return 0 on success.
*/
int qemu_socketpair(int domain, int type, int protocol, int sv[2]);
-#endif
int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);
/*
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 29a667ae3d..16f8a67f7e 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -310,6 +310,116 @@ bool qemu_socket_unselect(int sockfd, Error **errp)
return qemu_socket_select(sockfd, NULL, 0, errp);
}
+int qemu_socketpair(int domain, int type, int protocol, int sv[2])
+{
+struct sockaddr_un addr = {
+0,
+};
+socklen_t socklen;
+int listener = -1;
+int client = -1;
+int server = -1;
+g_autofree char *path = NULL;
+int tmpfd;
+u_long arg;
+int ret = -1;
+
+g_return_val_if_fail(sv != NULL, -1);
+
+addr.sun_family = AF_UNIX;
+socklen = sizeof(addr);
+
+tmpfd = g_file_open_tmp(NULL, &path, NULL);
+if (tmpfd == -1 || !path) {
+errno = EACCES;
+goto out;
+}
+
+close(tmpfd);
+
+if (strlen(path) >= sizeof(addr.sun_path)) {
+errno = EINVAL;
+goto out;
+}
+
+strncpy(addr.sun_path, path, sizeof(addr.sun_path) - 1);
+
+listener = socket(domain, type, protocol);
+if (listener == -1) {
+goto out;
+}
+
+if (DeleteFile(path) == 0 && GetLastError() != ERROR_FILE_NOT_FOUND) {
+errno = EACCES;
+goto out;
+}
+g_clear_pointer(&path, g_free);
+
+if (bind(listener, (struct sockaddr *)&addr, socklen) == -1) {
+goto out;
+}
+
+if (listen(listener, 1) == -1) {
+goto out;
+}
+
+client = socket(domain, type, protocol);
+if (client == -1) {
+goto out;
+}
+
+arg = 1;
+if (ioctlsocket(client, FIONBIO, &arg) != NO_ERROR) {
+goto out;
+}
+
+if (connect(client, (struct sockaddr *)&addr, socklen) == -1 &&
+WSAGetLastError() != WSAEWOULDBLOCK) {
+goto out;
+}
+
+server = accept(listener, NULL, NULL);
+if (server == -1) {
+goto out;
+}
+
+arg = 0;
+if (ioctlsocket(client, FIONBIO, &arg) != NO_ERROR) {
+goto out;
+}
+
+arg = 0;
+if (ioctlsocket(client, SIO_AF_UNIX_GETPEERPID, &arg) != NO_ERROR) {
+goto out;
+}
+
+if (arg != GetCurrentProcessId()) {
+errno = EPERM;
+goto out;
+}
+
+sv[0] = server;
+server = -1;
+sv[1] = client;
+client = -1;
+ret = 0;
+
+out:
+if (listener != -1) {
+close(listener);
+}
+if (client != -1) {
+close(client);
+}
+if (server != -1) {
+close(server);
+}
+if (path) {
+DeleteFile(path);
+}
+return ret;
+}
+
#undef connect
int qemu_connect_wrap(int sockfd, const struct sockaddr *addr,
socklen_t addrlen)
--
2.39.2
[PULL 24/25] qtest: enable vnc-display test on win32
From: Marc-André Lureau
Now that qtest_qmp_add_client() works on win32, we can enable the VNC
test.
Signed-off-by: Marc-André Lureau
Acked-by: Thomas Huth
Message-Id: <20230306122751.2355515-11-marcandre.lur...@redhat.com>
---
tests/qtest/vnc-display-test.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/tests/qtest/vnc-display-test.c b/tests/qtest/vnc-display-test.c
index e52a4326ec..f8933b0761 100644
--- a/tests/qtest/vnc-display-test.c
+++ b/tests/qtest/vnc-display-test.c
@@ -19,7 +19,7 @@ typedef struct Test {
GMainLoop *loop;
} Test;
-#if !defined(WIN32) && !defined(CONFIG_DARWIN)
+#if !defined(CONFIG_DARWIN)
static void on_vnc_error(VncConnection* self,
const char* msg)
@@ -38,10 +38,7 @@ static void on_vnc_auth_failure(VncConnection *self,
static bool
test_setup(Test *test)
{
-#ifdef WIN32
-g_test_skip("Not supported on Windows yet");
-return false;
-#elif defined(CONFIG_DARWIN)
+#if defined(CONFIG_DARWIN)
g_test_skip("Broken on Darwin");
return false;
#else
@@ -59,7 +56,12 @@ test_setup(Test *test)
g_signal_connect(test->conn, "vnc-auth-failure",
G_CALLBACK(on_vnc_auth_failure), NULL);
vnc_connection_set_auth_type(test->conn, VNC_CONNECTION_AUTH_NONE);
+
+#ifdef WIN32
+vnc_connection_open_fd(test->conn, _get_osfhandle(pair[0]));
+#else
vnc_connection_open_fd(test->conn, pair[0]);
+#endif
test->loop = g_main_loop_new(NULL, FALSE);
return true;
--
2.39.2
Re: [PULL 00/25] Win socket patches
Hi On Mon, Mar 13, 2023 at 3:44 PM wrote: > > From: Marc-André Lureau > > The following changes since commit 29c8a9e31a982874ce4e2c15f2bf82d5f8dc3517: > > Merge tag 'linux-user-for-8.0-pull-request' of > https://gitlab.com/laurent_vivier/qemu into staging (2023-03-12 10:57:00 > +) > > are available in the Git repository at: > > https://gitlab.com/marcandre.lureau/qemu.git tags/win-socket-pull-request > > for you to fetch changes up to 0006c18362cbe1e93587ac1e8ee965c08e6970f2: > > QMP/HMP: only actually implement getfd on CONFIG_POSIX (2023-03-13 15:41:32 > +0400) > > > QMP command to import win32 sockets > > > > Marc-André Lureau (25): > util: drop qemu_fork() > tests: use closesocket() > io: use closesocket() > tests: add test-error-report > error: add global &error_warn destination > win32/socket: introduce qemu_socket_select() helper > win32/socket: introduce qemu_socket_unselect() helper > aio: make aio_set_fd_poll() static to aio-posix.c > aio/win32: aio_set_fd_handler() only supports SOCKET > main-loop: remove qemu_fd_register(), win32/slirp/socket specific > slirp: unregister the win32 SOCKET > slirp: open-code qemu_socket_(un)select() > win32: avoid mixing SOCKET and file descriptor space > os-posix: remove useless ioctlsocket() define > win32: replace closesocket() with close() wrapper > tests: fix path separator, use g_build_filename() > char: do not double-close fd when failing to add client > tests/docker: fix a win32 error due to portability > osdep: implement qemu_socketpair() for win32 > qmp: 'add_client' actually expects sockets > monitor: release the lock before calling close() > qmp: add 'get-win32-socket' > libqtest: make qtest_qmp_add_client work on win32 > qtest: enable vnc-display test on win32 Would you consider those 3 patches as "features"? Is it too late to include before hard-feature freeze? The patches were ready for the soft-freeze deadline (I was expecting Paolo to send a PR last week, so I could quickly follow up with those patches in a PR) > QMP/HMP: only actually implement getfd on CONFIG_POSIX > > qapi/misc.json | 36 ++- > include/block/aio.h | 8 - > include/qapi/error.h | 6 + > include/qemu/main-loop.h | 2 - > include/qemu/osdep.h | 14 -- > include/qemu/sockets.h | 2 - > include/sysemu/os-posix.h| 3 - > include/sysemu/os-win32.h| 15 +- > tests/qtest/libqtest.h | 5 +- > backends/tpm/tpm_emulator.c | 6 +- > chardev/char.c | 2 - > crypto/afalg.c | 6 +- > hw/hyperv/syndbg.c | 4 +- > io/channel-socket.c | 8 +- > io/channel-watch.c | 10 +- > monitor/fds.c| 77 -- > monitor/hmp-cmds.c | 2 + > monitor/qmp-cmds.c | 7 + > net/dgram.c | 14 +- > net/slirp.c | 16 +- > net/socket.c | 22 +- > tests/qtest/libqtest.c | 26 +- > tests/qtest/microbit-test.c | 2 +- > tests/qtest/netdev-socket.c | 10 +- > tests/qtest/vnc-display-test.c | 12 +- > tests/unit/socket-helpers.c | 2 +- > tests/unit/test-error-report.c | 139 +++ > tests/unit/test-io-channel-command.c | 2 +- > util/aio-posix.c | 6 +- > util/aio-win32.c | 23 +- > util/error.c | 10 +- > util/main-loop.c | 11 - > util/oslib-posix.c | 70 -- > util/oslib-win32.c | 350 --- > util/qemu-sockets.c | 22 +- > hmp-commands.hx | 2 + > tests/docker/docker.py | 6 +- > tests/unit/meson.build | 1 + > 38 files changed, 701 insertions(+), 258 deletions(-) > create mode 100644 tests/unit/test-error-report.c > > -- > 2.39.2 > > -- Marc-André Lureau
[PULL v2 20/25] qmp: 'add_client' actually expects sockets
From: Marc-André Lureau
Whether it is SPICE, VNC, D-Bus, or the socket chardev, they all
actually expect a socket kind or will fail in different ways at runtime.
Throw an error early if the given 'add_client' fd is not a socket, and
close it to avoid leaks.
This allows to replace the close() call with a more correct & portable
closesocket() version.
(this will allow importing sockets on Windows with a specialized command
in the following patch, while keeping the remaining monitor associated
sockets/add_client code & usage untouched)
Signed-off-by: Marc-André Lureau
Reviewed-by: Philippe Mathieu-Daudé
Acked-by: Markus Armbruster
Message-Id: <20230306122751.2355515-6-marcandre.lur...@redhat.com>
---
qapi/misc.json | 3 +++
monitor/qmp-cmds.c | 7 +++
2 files changed, 10 insertions(+)
diff --git a/qapi/misc.json b/qapi/misc.json
index 27ef5a2b20..f0217cfba0 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -14,6 +14,9 @@
# Allow client connections for VNC, Spice and socket based
# character devices to be passed in to QEMU via SCM_RIGHTS.
#
+# If the FD associated with @fdname is not a socket, the command will fail and
+# the FD will be closed.
+#
# @protocol: protocol name. Valid names are "vnc", "spice", "@dbus-display" or
#the name of a character device (eg. from -chardev id=)
#
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 859012aef4..b0f948d337 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -14,6 +14,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/sockets.h"
#include "monitor-internal.h"
#include "monitor/qdev.h"
#include "monitor/qmp-helpers.h"
@@ -139,6 +140,12 @@ void qmp_add_client(const char *protocol, const char
*fdname,
return;
}
+if (!fd_is_socket(fd)) {
+error_setg(errp, "parameter @fdname must name a socket");
+close(fd);
+return;
+}
+
for (i = 0; i < ARRAY_SIZE(protocol_table); i++) {
if (!strcmp(protocol, protocol_table[i].name)) {
if (!protocol_table[i].add_client(fd, has_skipauth, skipauth,
--
2.39.2
[PULL v2 00/25] Win socket patches
From: Marc-André Lureau The following changes since commit 29c8a9e31a982874ce4e2c15f2bf82d5f8dc3517: Merge tag 'linux-user-for-8.0-pull-request' of https://gitlab.com/laurent_vivier/qemu into staging (2023-03-12 10:57:00 +) are available in the Git repository at: https://gitlab.com/marcandre.lureau/qemu.git tags/win-socket-pull-request for you to fetch changes up to 4bf21c7f748bee42b6f4692f8c37a11d1033b2d1: monitor: restrict command getfd to POSIX hosts (2023-03-13 15:46:09 +0400) QMP command to import win32 sockets Marc-André Lureau (25): util: drop qemu_fork() tests: use closesocket() io: use closesocket() tests: add test-error-report error: add global &error_warn destination win32/socket: introduce qemu_socket_select() helper win32/socket: introduce qemu_socket_unselect() helper aio: make aio_set_fd_poll() static to aio-posix.c aio/win32: aio_set_fd_handler() only supports SOCKET main-loop: remove qemu_fd_register(), win32/slirp/socket specific slirp: unregister the win32 SOCKET slirp: open-code qemu_socket_(un)select() win32: avoid mixing SOCKET and file descriptor space os-posix: remove useless ioctlsocket() define win32: replace closesocket() with close() wrapper tests: fix path separator, use g_build_filename() char: do not double-close fd when failing to add client tests/docker: fix a win32 error due to portability osdep: implement qemu_socketpair() for win32 qmp: 'add_client' actually expects sockets monitor: release the lock before calling close() qmp: add 'get-win32-socket' libqtest: make qtest_qmp_add_client work on win32 qtest: enable vnc-display test on win32 monitor: restrict command getfd to POSIX hosts qapi/misc.json | 36 ++- include/block/aio.h | 8 - include/qapi/error.h | 6 + include/qemu/main-loop.h | 2 - include/qemu/osdep.h | 14 -- include/qemu/sockets.h | 2 - include/sysemu/os-posix.h| 3 - include/sysemu/os-win32.h| 15 +- tests/qtest/libqtest.h | 5 +- backends/tpm/tpm_emulator.c | 6 +- chardev/char.c | 2 - crypto/afalg.c | 6 +- hw/hyperv/syndbg.c | 4 +- io/channel-socket.c | 8 +- io/channel-watch.c | 10 +- monitor/fds.c| 77 -- monitor/hmp-cmds.c | 2 + monitor/qmp-cmds.c | 7 + net/dgram.c | 14 +- net/slirp.c | 16 +- net/socket.c | 22 +- tests/qtest/libqtest.c | 26 +- tests/qtest/microbit-test.c | 2 +- tests/qtest/netdev-socket.c | 10 +- tests/qtest/vnc-display-test.c | 12 +- tests/unit/socket-helpers.c | 2 +- tests/unit/test-error-report.c | 139 +++ tests/unit/test-io-channel-command.c | 2 +- util/aio-posix.c | 6 +- util/aio-win32.c | 23 +- util/error.c | 10 +- util/main-loop.c | 11 - util/oslib-posix.c | 70 -- util/oslib-win32.c | 350 --- util/qemu-sockets.c | 22 +- hmp-commands.hx | 2 + tests/docker/docker.py | 6 +- tests/unit/meson.build | 1 + 38 files changed, 701 insertions(+), 258 deletions(-) create mode 100644 tests/unit/test-error-report.c -- 2.39.2
[PULL v2 08/25] aio: make aio_set_fd_poll() static to aio-posix.c
From: Marc-André Lureau
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-9-marcandre.lur...@redhat.com>
---
include/block/aio.h | 8
util/aio-posix.c| 6 +++---
util/aio-win32.c| 7 ---
3 files changed, 3 insertions(+), 18 deletions(-)
diff --git a/include/block/aio.h b/include/block/aio.h
index 8fba6a3584..543717f294 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -482,14 +482,6 @@ void aio_set_fd_handler(AioContext *ctx,
IOHandler *io_poll_ready,
void *opaque);
-/* Set polling begin/end callbacks for a file descriptor that has already been
- * registered with aio_set_fd_handler. Do nothing if the file descriptor is
- * not registered.
- */
-void aio_set_fd_poll(AioContext *ctx, int fd,
- IOHandler *io_poll_begin,
- IOHandler *io_poll_end);
-
/* Register an event notifier and associated callbacks. Behaves very similarly
* to event_notifier_set_handler. Unlike event_notifier_set_handler, these
callbacks
* will be invoked when using aio_poll().
diff --git a/util/aio-posix.c b/util/aio-posix.c
index 6cc6256d53..a8be940f76 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -180,9 +180,9 @@ void aio_set_fd_handler(AioContext *ctx,
}
}
-void aio_set_fd_poll(AioContext *ctx, int fd,
- IOHandler *io_poll_begin,
- IOHandler *io_poll_end)
+static void aio_set_fd_poll(AioContext *ctx, int fd,
+IOHandler *io_poll_begin,
+IOHandler *io_poll_end)
{
AioHandler *node = find_aio_handler(ctx, fd);
diff --git a/util/aio-win32.c b/util/aio-win32.c
index be5136e486..74d63fa21e 100644
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -125,13 +125,6 @@ void aio_set_fd_handler(AioContext *ctx,
aio_notify(ctx);
}
-void aio_set_fd_poll(AioContext *ctx, int fd,
- IOHandler *io_poll_begin,
- IOHandler *io_poll_end)
-{
-/* Not implemented */
-}
-
void aio_set_event_notifier(AioContext *ctx,
EventNotifier *e,
bool is_external,
--
2.39.2
[PULL 01/25] util: drop qemu_fork()
From: Marc-André Lureau
Fortunately, qemu_fork() is no longer used since commit
a95570e3e4d6 ("io/command: use glib GSpawn, instead of open-coding
fork/exec"). (GSpawn uses posix_spawn() whenever possible instead)
Signed-off-by: Marc-André Lureau
Reviewed-by: Daniel P. Berrangé
Message-Id: <20230221124802.4103554-2-marcandre.lur...@redhat.com>
---
include/qemu/osdep.h | 14 -
util/oslib-posix.c | 70
util/oslib-win32.c | 9 --
3 files changed, 93 deletions(-)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 88c9facbf2..f68b5d8708 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -665,20 +665,6 @@ void qemu_prealloc_mem(int fd, char *area, size_t sz, int
max_threads,
*/
char *qemu_get_pid_name(pid_t pid);
-/**
- * qemu_fork:
- *
- * A version of fork that avoids signal handler race
- * conditions that can lead to child process getting
- * signals that are otherwise only expected by the
- * parent. It also resets all signal handlers to the
- * default settings.
- *
- * Returns 0 to child process, pid number to parent
- * or -1 on failure.
- */
-pid_t qemu_fork(Error **errp);
-
/* Using intptr_t ensures that qemu_*_page_mask is sign-extended even
* when intptr_t is 32-bit and we are aligning a long long.
*/
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 77d882e681..760390b31e 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -583,76 +583,6 @@ char *qemu_get_pid_name(pid_t pid)
}
-pid_t qemu_fork(Error **errp)
-{
-sigset_t oldmask, newmask;
-struct sigaction sig_action;
-int saved_errno;
-pid_t pid;
-
-/*
- * Need to block signals now, so that child process can safely
- * kill off caller's signal handlers without a race.
- */
-sigfillset(&newmask);
-if (pthread_sigmask(SIG_SETMASK, &newmask, &oldmask) != 0) {
-error_setg_errno(errp, errno,
- "cannot block signals");
-return -1;
-}
-
-pid = fork();
-saved_errno = errno;
-
-if (pid < 0) {
-/* attempt to restore signal mask, but ignore failure, to
- * avoid obscuring the fork failure */
-(void)pthread_sigmask(SIG_SETMASK, &oldmask, NULL);
-error_setg_errno(errp, saved_errno,
- "cannot fork child process");
-errno = saved_errno;
-return -1;
-} else if (pid) {
-/* parent process */
-
-/* Restore our original signal mask now that the child is
- * safely running. Only documented failures are EFAULT (not
- * possible, since we are using just-grabbed mask) or EINVAL
- * (not possible, since we are using correct arguments). */
-(void)pthread_sigmask(SIG_SETMASK, &oldmask, NULL);
-} else {
-/* child process */
-size_t i;
-
-/* Clear out all signal handlers from parent so nothing
- * unexpected can happen in our child once we unblock
- * signals */
-sig_action.sa_handler = SIG_DFL;
-sig_action.sa_flags = 0;
-sigemptyset(&sig_action.sa_mask);
-
-for (i = 1; i < NSIG; i++) {
-/* Only possible errors are EFAULT or EINVAL The former
- * won't happen, the latter we expect, so no need to check
- * return value */
-(void)sigaction(i, &sig_action, NULL);
-}
-
-/* Unmask all signals in child, since we've no idea what the
- * caller's done with their signal mask and don't want to
- * propagate that to children */
-sigemptyset(&newmask);
-if (pthread_sigmask(SIG_SETMASK, &newmask, NULL) != 0) {
-Error *local_err = NULL;
-error_setg_errno(&local_err, errno,
- "cannot unblock signals");
-error_report_err(local_err);
-_exit(1);
-}
-}
-return pid;
-}
-
void *qemu_alloc_stack(size_t *sz)
{
void *ptr, *guardpage;
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 07ade41800..528c9ee156 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -283,15 +283,6 @@ char *qemu_get_pid_name(pid_t pid)
}
-pid_t qemu_fork(Error **errp)
-{
-errno = ENOSYS;
-error_setg_errno(errp, errno,
- "cannot fork child process");
-return -1;
-}
-
-
#undef connect
int qemu_connect_wrap(int sockfd, const struct sockaddr *addr,
socklen_t addrlen)
--
2.39.2
[PULL v2 12/25] slirp: open-code qemu_socket_(un)select()
From: Marc-André Lureau
We are about to make the QEMU socket API use file-descriptor space only,
but libslirp gives us SOCKET as fd, still.
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-14-marcandre.lur...@redhat.com>
---
net/slirp.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/net/slirp.c b/net/slirp.c
index a7c35778a6..c33b3e02e7 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -251,16 +251,20 @@ static void net_slirp_register_poll_fd(int fd, void
*opaque)
#ifdef WIN32
AioContext *ctxt = qemu_get_aio_context();
-qemu_socket_select(fd, event_notifier_get_handle(&ctxt->notifier),
+if (WSAEventSelect(fd, event_notifier_get_handle(&ctxt->notifier),
FD_READ | FD_ACCEPT | FD_CLOSE |
- FD_CONNECT | FD_WRITE | FD_OOB, NULL);
+ FD_CONNECT | FD_WRITE | FD_OOB) != 0) {
+error_setg_win32(&error_warn, WSAGetLastError(), "failed to
WSAEventSelect()");
+}
#endif
}
static void net_slirp_unregister_poll_fd(int fd, void *opaque)
{
#ifdef WIN32
-qemu_socket_unselect(fd, NULL);
+if (WSAEventSelect(fd, NULL, 0) != 0) {
+error_setg_win32(&error_warn, WSAGetLastError(), "failed to
WSAEventSelect()");
+}
#endif
}
--
2.39.2
[PULL v2 14/25] os-posix: remove useless ioctlsocket() define
From: Marc-André Lureau The API is specific to win32. Signed-off-by: Marc-André Lureau Reviewed-by: Daniel P. Berrangé Message-Id: <20230221124802.4103554-16-marcandre.lur...@redhat.com> --- include/sysemu/os-posix.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h index 58de7c994d..378213fc86 100644 --- a/include/sysemu/os-posix.h +++ b/include/sysemu/os-posix.h @@ -52,7 +52,6 @@ void os_setup_post(void); int os_mlock(void); #define closesocket(s) close(s) -#define ioctlsocket(s, r, v) ioctl(s, r, v) int os_set_daemonize(bool d); bool is_daemonized(void); -- 2.39.2
[PULL v2 02/25] tests: use closesocket()
From: Marc-André Lureau
Because they are actually sockets...
Signed-off-by: Marc-André Lureau
Reviewed-by: Thomas Huth
Message-Id: <20230221124802.4103554-3-marcandre.lur...@redhat.com>
---
tests/unit/socket-helpers.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/unit/socket-helpers.c b/tests/unit/socket-helpers.c
index eecadf3a3c..914b3aa0cf 100644
--- a/tests/unit/socket-helpers.c
+++ b/tests/unit/socket-helpers.c
@@ -117,13 +117,13 @@ static int socket_can_bind_connect(const char *hostname,
int family)
cleanup:
if (afd != -1) {
-close(afd);
+closesocket(afd);
}
if (cfd != -1) {
-close(cfd);
+closesocket(cfd);
}
if (lfd != -1) {
-close(lfd);
+closesocket(lfd);
}
if (res) {
freeaddrinfo(res);
--
2.39.2
[PULL v2 18/25] tests/docker: fix a win32 error due to portability
From: Marc-André Lureau
docker.py is run during configure, and produces an error: No module
named 'pwd'.
Use a more portable and recommended alternative to lookup the user
"login name".
Signed-off-by: Marc-André Lureau
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <20230306122751.2355515-4-marcandre.lur...@redhat.com>
---
tests/docker/docker.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index 3a1ed7cb18..688ef62989 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -23,10 +23,10 @@
import tempfile
import re
import signal
+import getpass
from tarfile import TarFile, TarInfo
from io import StringIO, BytesIO
from shutil import copy, rmtree
-from pwd import getpwuid
from datetime import datetime, timedelta
@@ -316,7 +316,7 @@ def build_image(self, tag, docker_dir, dockerfile,
if user:
uid = os.getuid()
-uname = getpwuid(uid).pw_name
+uname = getpass.getuser()
tmp_df.write("\n")
tmp_df.write("RUN id %s 2>/dev/null || useradd -u %d -U %s" %
(uname, uid, uname))
@@ -570,7 +570,7 @@ def run(self, args, argv):
if args.user:
uid = os.getuid()
-uname = getpwuid(uid).pw_name
+uname = getpass.getuser()
df.write("\n")
df.write("RUN id %s 2>/dev/null || useradd -u %d -U %s" %
(uname, uid, uname))
--
2.39.2
[PULL v2 21/25] monitor: release the lock before calling close()
From: Marc-André Lureau
As per comment, presumably to avoid syscall in critical section.
Fixes: 0210c3b39bef08 ("monitor: Use LOCK_GUARD macros")
Signed-off-by: Marc-André Lureau
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <20230306122751.2355515-7-marcandre.lur...@redhat.com>
---
monitor/fds.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/monitor/fds.c b/monitor/fds.c
index 26b39a0ce6..7daf1064e1 100644
--- a/monitor/fds.c
+++ b/monitor/fds.c
@@ -80,7 +80,8 @@ void qmp_getfd(const char *fdname, Error **errp)
return;
}
-QEMU_LOCK_GUARD(&cur_mon->mon_lock);
+/* See close() call below. */
+qemu_mutex_lock(&cur_mon->mon_lock);
QLIST_FOREACH(monfd, &cur_mon->fds, next) {
if (strcmp(monfd->name, fdname) != 0) {
continue;
@@ -88,6 +89,7 @@ void qmp_getfd(const char *fdname, Error **errp)
tmp_fd = monfd->fd;
monfd->fd = fd;
+qemu_mutex_unlock(&cur_mon->mon_lock);
/* Make sure close() is outside critical section */
close(tmp_fd);
return;
@@ -98,6 +100,7 @@ void qmp_getfd(const char *fdname, Error **errp)
monfd->fd = fd;
QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
+qemu_mutex_unlock(&cur_mon->mon_lock);
}
void qmp_closefd(const char *fdname, Error **errp)
--
2.39.2
[PULL v2 06/25] win32/socket: introduce qemu_socket_select() helper
From: Marc-André Lureau
This is a wrapper for WSAEventSelect, with Error handling. By default,
it will produce a warning, so callers don't have to be modified
now, and yet we can spot potential mis-use.
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-7-marcandre.lur...@redhat.com>
---
include/sysemu/os-win32.h | 5 +
io/channel-socket.c | 4 ++--
io/channel-watch.c| 6 +++---
util/aio-win32.c | 2 +-
util/main-loop.c | 6 +++---
util/oslib-win32.c| 17 -
6 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 97d0243aee..9f842ae643 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -29,6 +29,7 @@
#include
#include
#include
+#include "qemu/typedefs.h"
#ifdef HAVE_AFUNIX_H
#include
@@ -164,6 +165,10 @@ static inline void qemu_funlockfile(FILE *f)
#endif
}
+/* Helper for WSAEventSelect, to report errors */
+bool qemu_socket_select(SOCKET s, WSAEVENT hEventObject,
+long lNetworkEvents, Error **errp);
+
/* We wrap all the sockets functions so that we can
* set errno based on WSAGetLastError()
*/
diff --git a/io/channel-socket.c b/io/channel-socket.c
index 2040297d2b..0bc29c4808 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -442,7 +442,7 @@ static void qio_channel_socket_finalize(Object *obj)
}
}
#ifdef WIN32
-WSAEventSelect(ioc->fd, NULL, 0);
+qemu_socket_select(ioc->fd, NULL, 0, NULL);
#endif
closesocket(ioc->fd);
ioc->fd = -1;
@@ -846,7 +846,7 @@ qio_channel_socket_close(QIOChannel *ioc,
if (sioc->fd != -1) {
#ifdef WIN32
-WSAEventSelect(sioc->fd, NULL, 0);
+qemu_socket_select(sioc->fd, NULL, 0, NULL);
#endif
if (qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_LISTEN)) {
socket_listen_cleanup(sioc->fd, errp);
diff --git a/io/channel-watch.c b/io/channel-watch.c
index ad7c568a84..6ac41009fa 100644
--- a/io/channel-watch.c
+++ b/io/channel-watch.c
@@ -281,9 +281,9 @@ GSource *qio_channel_create_socket_watch(QIOChannel *ioc,
GSource *source;
QIOChannelSocketSource *ssource;
-WSAEventSelect(socket, ioc->event,
- FD_READ | FD_ACCEPT | FD_CLOSE |
- FD_CONNECT | FD_WRITE | FD_OOB);
+qemu_socket_select(socket, ioc->event,
+ FD_READ | FD_ACCEPT | FD_CLOSE |
+ FD_CONNECT | FD_WRITE | FD_OOB, NULL);
source = g_source_new(&qio_channel_socket_source_funcs,
sizeof(QIOChannelSocketSource));
diff --git a/util/aio-win32.c b/util/aio-win32.c
index 80cfe012ad..be5136e486 100644
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -115,7 +115,7 @@ void aio_set_fd_handler(AioContext *ctx,
QLIST_INSERT_HEAD_RCU(&ctx->aio_handlers, node, node);
event = event_notifier_get_handle(&ctx->notifier);
-WSAEventSelect(node->pfd.fd, event, bitmask);
+qemu_socket_select(node->pfd.fd, event, bitmask, NULL);
}
if (old_node) {
aio_remove_fd_handler(ctx, old_node);
diff --git a/util/main-loop.c b/util/main-loop.c
index 3c0f525192..16e837fb12 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -416,9 +416,9 @@ void qemu_del_wait_object(HANDLE handle, WaitObjectFunc
*func, void *opaque)
void qemu_fd_register(int fd)
{
-WSAEventSelect(fd, event_notifier_get_handle(&qemu_aio_context->notifier),
- FD_READ | FD_ACCEPT | FD_CLOSE |
- FD_CONNECT | FD_WRITE | FD_OOB);
+qemu_socket_select(fd,
event_notifier_get_handle(&qemu_aio_context->notifier),
+ FD_READ | FD_ACCEPT | FD_CLOSE |
+ FD_CONNECT | FD_WRITE | FD_OOB, NULL);
}
static int pollfds_fill(GArray *pollfds, fd_set *rfds, fd_set *wfds,
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 528c9ee156..df752fc762 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -180,7 +180,7 @@ static int socket_error(void)
void qemu_socket_set_block(int fd)
{
unsigned long opt = 0;
-WSAEventSelect(fd, NULL, 0);
+qemu_socket_select(fd, NULL, 0, NULL);
ioctlsocket(fd, FIONBIO, &opt);
}
@@ -283,6 +283,21 @@ char *qemu_get_pid_name(pid_t pid)
}
+bool qemu_socket_select(SOCKET s, WSAEVENT hEventObject,
+long lNetworkEvents, Error **errp)
+{
+if (errp == NULL) {
+errp = &error_warn;
+}
+
+if (WSAEventSelect(s, hEventObject, lNetworkEvents) != 0) {
+error_setg_win32(errp, WSAGetLastError(), "failed to
WSAEventSelect()");
+return false;
+}
+
+return true;
+}
+
#undef connect
int qemu_connect_wrap(int sockfd, const struct sockaddr *addr,
socklen_t addrlen)
--
2.39.2
[PULL v2 05/25] error: add global &error_warn destination
From: Marc-André Lureau
This can help debugging issues or develop, when error handling is
introduced.
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-6-marcandre.lur...@redhat.com>
---
include/qapi/error.h | 6 ++
tests/unit/test-error-report.c | 18 ++
util/error.c | 10 +++---
3 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/include/qapi/error.h b/include/qapi/error.h
index d798faeec3..f21a231bb1 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -519,6 +519,12 @@ static inline void
error_propagator_cleanup(ErrorPropagator *prop)
G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(ErrorPropagator, error_propagator_cleanup);
+/*
+ * Special error destination to warn on error.
+ * See error_setg() and error_propagate() for details.
+ */
+extern Error *error_warn;
+
/*
* Special error destination to abort on error.
* See error_setg() and error_propagate() for details.
diff --git a/tests/unit/test-error-report.c b/tests/unit/test-error-report.c
index b09650687b..54319c86c9 100644
--- a/tests/unit/test-error-report.c
+++ b/tests/unit/test-error-report.c
@@ -12,6 +12,7 @@
#include
#include "qemu/error-report.h"
+#include "qapi/error.h"
static void
test_error_report_simple(void)
@@ -103,6 +104,22 @@ test_error_report_timestamp(void)
");
}
+static void
+test_error_warn(void)
+{
+if (g_test_subprocess()) {
+error_setg(&error_warn, "Testing &error_warn");
+return;
+}
+
+g_test_trap_subprocess(NULL, 0, 0);
+g_test_trap_assert_passed();
+g_test_trap_assert_stderr("\
+test-error-report: warning: Testing &error_warn*\
+");
+}
+
+
int
main(int argc, char *argv[])
{
@@ -116,6 +133,7 @@ main(int argc, char *argv[])
g_test_add_func("/error-report/glog", test_error_report_glog);
g_test_add_func("/error-report/once", test_error_report_once);
g_test_add_func("/error-report/timestamp", test_error_report_timestamp);
+g_test_add_func("/error-report/warn", test_error_warn);
return g_test_run();
}
diff --git a/util/error.c b/util/error.c
index 1e7af665b8..5537245da6 100644
--- a/util/error.c
+++ b/util/error.c
@@ -27,8 +27,9 @@ struct Error
Error *error_abort;
Error *error_fatal;
+Error *error_warn;
-static void error_handle_fatal(Error **errp, Error *err)
+static void error_handle(Error **errp, Error *err)
{
if (errp == &error_abort) {
fprintf(stderr, "Unexpected error in %s() at %s:%d:\n",
@@ -43,6 +44,9 @@ static void error_handle_fatal(Error **errp, Error *err)
error_report_err(err);
exit(1);
}
+if (errp == &error_warn) {
+warn_report_err(err);
+}
}
G_GNUC_PRINTF(6, 0)
@@ -71,7 +75,7 @@ static void error_setv(Error **errp,
err->line = line;
err->func = func;
-error_handle_fatal(errp, err);
+error_handle(errp, err);
*errp = err;
errno = saved_errno;
@@ -284,7 +288,7 @@ void error_propagate(Error **dst_errp, Error *local_err)
if (!local_err) {
return;
}
-error_handle_fatal(dst_errp, local_err);
+error_handle(dst_errp, local_err);
if (dst_errp && !*dst_errp) {
*dst_errp = local_err;
} else {
--
2.39.2
[PULL 03/25] io: use closesocket()
From: Marc-André Lureau
Because they are actually sockets...
Signed-off-by: Marc-André Lureau
Reviewed-by: Thomas Huth
Message-Id: <20230221124802.4103554-4-marcandre.lur...@redhat.com>
---
io/channel-socket.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/io/channel-socket.c b/io/channel-socket.c
index 7aca84f61a..2040297d2b 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -159,7 +159,7 @@ int qio_channel_socket_connect_sync(QIOChannelSocket *ioc,
trace_qio_channel_socket_connect_complete(ioc, fd);
if (qio_channel_socket_set_fd(ioc, fd, errp) < 0) {
-close(fd);
+closesocket(fd);
return -1;
}
@@ -233,7 +233,7 @@ int qio_channel_socket_listen_sync(QIOChannelSocket *ioc,
trace_qio_channel_socket_listen_complete(ioc, fd);
if (qio_channel_socket_set_fd(ioc, fd, errp) < 0) {
-close(fd);
+closesocket(fd);
return -1;
}
qio_channel_set_feature(QIO_CHANNEL(ioc), QIO_CHANNEL_FEATURE_LISTEN);
@@ -310,7 +310,7 @@ int qio_channel_socket_dgram_sync(QIOChannelSocket *ioc,
trace_qio_channel_socket_dgram_complete(ioc, fd);
if (qio_channel_socket_set_fd(ioc, fd, errp) < 0) {
-close(fd);
+closesocket(fd);
return -1;
}
--
2.39.2
[PULL 22/25] qmp: add 'get-win32-socket'
From: Marc-André Lureau
A process with enough capabilities can duplicate a socket to QEMU. Add a
QMP command to import it and add it to the monitor fd list, so it can be
later used by other commands.
Signed-off-by: Marc-André Lureau
Acked-by: Markus Armbruster
Message-Id: <20230306122751.2355515-9-marcandre.lur...@redhat.com>
---
qapi/misc.json | 31
monitor/fds.c | 76 +++---
2 files changed, 91 insertions(+), 16 deletions(-)
diff --git a/qapi/misc.json b/qapi/misc.json
index f0217cfba0..5ef6286af3 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -275,6 +275,37 @@
##
{ 'command': 'getfd', 'data': {'fdname': 'str'} }
+##
+# @get-win32-socket:
+#
+# Add a socket that was duplicated to QEMU process with
+# WSADuplicateSocketW() via WSASocket() & WSAPROTOCOL_INFOW structure
+# and assign it a name (the SOCKET is associated with a CRT file
+# descriptor)
+#
+# @info: the WSAPROTOCOL_INFOW structure (encoded in base64)
+#
+# @fdname: file descriptor name
+#
+# Returns: Nothing on success
+#
+# Since: 8.0
+#
+# Notes: If @fdname already exists, the file descriptor assigned to
+#it will be closed and replaced by the received file
+#descriptor.
+#
+#The 'closefd' command can be used to explicitly close the
+#file descriptor when it is no longer needed.
+#
+# Example:
+#
+# -> { "execute": "get-win32-socket", "arguments": { "info": "abcd123..",
fdname": "skclient" } }
+# <- { "return": {} }
+#
+##
+{ 'command': 'get-win32-socket', 'data': {'info': 'str', 'fdname': 'str'},
'if': 'CONFIG_WIN32' }
+
##
# @closefd:
#
diff --git a/monitor/fds.c b/monitor/fds.c
index 7daf1064e1..9ed4197358 100644
--- a/monitor/fds.c
+++ b/monitor/fds.c
@@ -61,46 +61,55 @@ struct MonFdset {
static QemuMutex mon_fdsets_lock;
static QLIST_HEAD(, MonFdset) mon_fdsets;
-void qmp_getfd(const char *fdname, Error **errp)
+static bool monitor_add_fd(Monitor *mon, int fd, const char *fdname, Error
**errp)
{
-Monitor *cur_mon = monitor_cur();
mon_fd_t *monfd;
-int fd, tmp_fd;
-
-fd = qemu_chr_fe_get_msgfd(&cur_mon->chr);
-if (fd == -1) {
-error_setg(errp, "No file descriptor supplied via SCM_RIGHTS");
-return;
-}
if (qemu_isdigit(fdname[0])) {
close(fd);
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
"a name not starting with a digit");
-return;
+return false;
}
/* See close() call below. */
-qemu_mutex_lock(&cur_mon->mon_lock);
-QLIST_FOREACH(monfd, &cur_mon->fds, next) {
+qemu_mutex_lock(&mon->mon_lock);
+QLIST_FOREACH(monfd, &mon->fds, next) {
+int tmp_fd;
+
if (strcmp(monfd->name, fdname) != 0) {
continue;
}
tmp_fd = monfd->fd;
monfd->fd = fd;
-qemu_mutex_unlock(&cur_mon->mon_lock);
+qemu_mutex_unlock(&mon->mon_lock);
/* Make sure close() is outside critical section */
close(tmp_fd);
-return;
+return true;
}
monfd = g_new0(mon_fd_t, 1);
monfd->name = g_strdup(fdname);
monfd->fd = fd;
-QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
-qemu_mutex_unlock(&cur_mon->mon_lock);
+QLIST_INSERT_HEAD(&mon->fds, monfd, next);
+qemu_mutex_unlock(&mon->mon_lock);
+return true;
+}
+
+void qmp_getfd(const char *fdname, Error **errp)
+{
+Monitor *cur_mon = monitor_cur();
+int fd;
+
+fd = qemu_chr_fe_get_msgfd(&cur_mon->chr);
+if (fd == -1) {
+error_setg(errp, "No file descriptor supplied via SCM_RIGHTS");
+return;
+}
+
+monitor_add_fd(cur_mon, fd, fdname, errp);
}
void qmp_closefd(const char *fdname, Error **errp)
@@ -214,6 +223,41 @@ error:
return NULL;
}
+#ifdef WIN32
+void qmp_get_win32_socket(const char *infos, const char *fdname, Error **errp)
+{
+g_autofree WSAPROTOCOL_INFOW *info = NULL;
+gsize len;
+SOCKET sk;
+int fd;
+
+info = (void *)g_base64_decode(infos, &len);
+if (len != sizeof(*info)) {
+error_setg(errp, "Invalid WSAPROTOCOL_INFOW value");
+return;
+}
+
+sk = WSASocketW(FROM_PROTOCOL_INFO,
+FROM_PROTOCOL_INFO,
+FROM_PROTOCOL_INFO,
+info, 0, 0);
+if (sk == INVALID_SOCKET) {
+error_setg_win32(errp, WSAGetLastError(), "Couldn't import socket");
+return;
+}
+
+fd = _open_osfhandle(sk, _O_BINARY);
+if (fd < 0) {
+error_setg_errno(errp, errno, "Failed to associate a FD with the
SOCKET");
+closesocket(sk);
+return;
+}
+
+monitor_add_fd(monitor_cur(), fd, fdname, errp);
+}
+#endif
+
+
void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
{
MonFdset *mon_fdset;
--
2.39.2
[PULL v2 11/25] slirp: unregister the win32 SOCKET
From: Marc-André Lureau
Presumably, this is what should happen when the SOCKET is to be removed.
(it probably worked until now because closesocket() does it implicitly,
but we never now how the slirp library could use the SOCKET later)
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-13-marcandre.lur...@redhat.com>
---
net/slirp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/slirp.c b/net/slirp.c
index 0730a935ba..a7c35778a6 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -259,7 +259,9 @@ static void net_slirp_register_poll_fd(int fd, void *opaque)
static void net_slirp_unregister_poll_fd(int fd, void *opaque)
{
-/* no qemu_fd_unregister */
+#ifdef WIN32
+qemu_socket_unselect(fd, NULL);
+#endif
}
static void net_slirp_notify(void *opaque)
--
2.39.2
[PULL v2 22/25] qmp: add 'get-win32-socket'
From: Marc-André Lureau
A process with enough capabilities can duplicate a socket to QEMU. Add a
QMP command to import it and add it to the monitor fd list, so it can be
later used by other commands.
Signed-off-by: Marc-André Lureau
Acked-by: Markus Armbruster
Message-Id: <20230306122751.2355515-9-marcandre.lur...@redhat.com>
---
qapi/misc.json | 31
monitor/fds.c | 76 +++---
2 files changed, 91 insertions(+), 16 deletions(-)
diff --git a/qapi/misc.json b/qapi/misc.json
index f0217cfba0..5ef6286af3 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -275,6 +275,37 @@
##
{ 'command': 'getfd', 'data': {'fdname': 'str'} }
+##
+# @get-win32-socket:
+#
+# Add a socket that was duplicated to QEMU process with
+# WSADuplicateSocketW() via WSASocket() & WSAPROTOCOL_INFOW structure
+# and assign it a name (the SOCKET is associated with a CRT file
+# descriptor)
+#
+# @info: the WSAPROTOCOL_INFOW structure (encoded in base64)
+#
+# @fdname: file descriptor name
+#
+# Returns: Nothing on success
+#
+# Since: 8.0
+#
+# Notes: If @fdname already exists, the file descriptor assigned to
+#it will be closed and replaced by the received file
+#descriptor.
+#
+#The 'closefd' command can be used to explicitly close the
+#file descriptor when it is no longer needed.
+#
+# Example:
+#
+# -> { "execute": "get-win32-socket", "arguments": { "info": "abcd123..",
fdname": "skclient" } }
+# <- { "return": {} }
+#
+##
+{ 'command': 'get-win32-socket', 'data': {'info': 'str', 'fdname': 'str'},
'if': 'CONFIG_WIN32' }
+
##
# @closefd:
#
diff --git a/monitor/fds.c b/monitor/fds.c
index 7daf1064e1..9ed4197358 100644
--- a/monitor/fds.c
+++ b/monitor/fds.c
@@ -61,46 +61,55 @@ struct MonFdset {
static QemuMutex mon_fdsets_lock;
static QLIST_HEAD(, MonFdset) mon_fdsets;
-void qmp_getfd(const char *fdname, Error **errp)
+static bool monitor_add_fd(Monitor *mon, int fd, const char *fdname, Error
**errp)
{
-Monitor *cur_mon = monitor_cur();
mon_fd_t *monfd;
-int fd, tmp_fd;
-
-fd = qemu_chr_fe_get_msgfd(&cur_mon->chr);
-if (fd == -1) {
-error_setg(errp, "No file descriptor supplied via SCM_RIGHTS");
-return;
-}
if (qemu_isdigit(fdname[0])) {
close(fd);
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "fdname",
"a name not starting with a digit");
-return;
+return false;
}
/* See close() call below. */
-qemu_mutex_lock(&cur_mon->mon_lock);
-QLIST_FOREACH(monfd, &cur_mon->fds, next) {
+qemu_mutex_lock(&mon->mon_lock);
+QLIST_FOREACH(monfd, &mon->fds, next) {
+int tmp_fd;
+
if (strcmp(monfd->name, fdname) != 0) {
continue;
}
tmp_fd = monfd->fd;
monfd->fd = fd;
-qemu_mutex_unlock(&cur_mon->mon_lock);
+qemu_mutex_unlock(&mon->mon_lock);
/* Make sure close() is outside critical section */
close(tmp_fd);
-return;
+return true;
}
monfd = g_new0(mon_fd_t, 1);
monfd->name = g_strdup(fdname);
monfd->fd = fd;
-QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
-qemu_mutex_unlock(&cur_mon->mon_lock);
+QLIST_INSERT_HEAD(&mon->fds, monfd, next);
+qemu_mutex_unlock(&mon->mon_lock);
+return true;
+}
+
+void qmp_getfd(const char *fdname, Error **errp)
+{
+Monitor *cur_mon = monitor_cur();
+int fd;
+
+fd = qemu_chr_fe_get_msgfd(&cur_mon->chr);
+if (fd == -1) {
+error_setg(errp, "No file descriptor supplied via SCM_RIGHTS");
+return;
+}
+
+monitor_add_fd(cur_mon, fd, fdname, errp);
}
void qmp_closefd(const char *fdname, Error **errp)
@@ -214,6 +223,41 @@ error:
return NULL;
}
+#ifdef WIN32
+void qmp_get_win32_socket(const char *infos, const char *fdname, Error **errp)
+{
+g_autofree WSAPROTOCOL_INFOW *info = NULL;
+gsize len;
+SOCKET sk;
+int fd;
+
+info = (void *)g_base64_decode(infos, &len);
+if (len != sizeof(*info)) {
+error_setg(errp, "Invalid WSAPROTOCOL_INFOW value");
+return;
+}
+
+sk = WSASocketW(FROM_PROTOCOL_INFO,
+FROM_PROTOCOL_INFO,
+FROM_PROTOCOL_INFO,
+info, 0, 0);
+if (sk == INVALID_SOCKET) {
+error_setg_win32(errp, WSAGetLastError(), "Couldn't import socket");
+return;
+}
+
+fd = _open_osfhandle(sk, _O_BINARY);
+if (fd < 0) {
+error_setg_errno(errp, errno, "Failed to associate a FD with the
SOCKET");
+closesocket(sk);
+return;
+}
+
+monitor_add_fd(monitor_cur(), fd, fdname, errp);
+}
+#endif
+
+
void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
{
MonFdset *mon_fdset;
--
2.39.2
[PULL v2 09/25] aio/win32: aio_set_fd_handler() only supports SOCKET
From: Marc-André Lureau
Let's check if the argument is actually a SOCKET, else report an error
and return.
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-10-marcandre.lur...@redhat.com>
---
util/aio-win32.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/util/aio-win32.c b/util/aio-win32.c
index 74d63fa21e..08e8f5615d 100644
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -22,6 +22,7 @@
#include "qemu/sockets.h"
#include "qapi/error.h"
#include "qemu/rcu_queue.h"
+#include "qemu/error-report.h"
struct AioHandler {
EventNotifier *e;
@@ -70,10 +71,14 @@ void aio_set_fd_handler(AioContext *ctx,
IOHandler *io_poll_ready,
void *opaque)
{
-/* fd is a SOCKET in our case */
AioHandler *old_node;
AioHandler *node = NULL;
+if (!fd_is_socket(fd)) {
+error_report("fd=%d is not a socket, AIO implementation is missing",
fd);
+return;
+}
+
qemu_lockcnt_lock(&ctx->list_lock);
QLIST_FOREACH(old_node, &ctx->aio_handlers, node) {
if (old_node->pfd.fd == fd && !old_node->deleted) {
--
2.39.2
[PULL v2 01/25] util: drop qemu_fork()
From: Marc-André Lureau
Fortunately, qemu_fork() is no longer used since commit
a95570e3e4d6 ("io/command: use glib GSpawn, instead of open-coding
fork/exec"). (GSpawn uses posix_spawn() whenever possible instead)
Signed-off-by: Marc-André Lureau
Reviewed-by: Daniel P. Berrangé
Message-Id: <20230221124802.4103554-2-marcandre.lur...@redhat.com>
---
include/qemu/osdep.h | 14 -
util/oslib-posix.c | 70
util/oslib-win32.c | 9 --
3 files changed, 93 deletions(-)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 88c9facbf2..f68b5d8708 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -665,20 +665,6 @@ void qemu_prealloc_mem(int fd, char *area, size_t sz, int
max_threads,
*/
char *qemu_get_pid_name(pid_t pid);
-/**
- * qemu_fork:
- *
- * A version of fork that avoids signal handler race
- * conditions that can lead to child process getting
- * signals that are otherwise only expected by the
- * parent. It also resets all signal handlers to the
- * default settings.
- *
- * Returns 0 to child process, pid number to parent
- * or -1 on failure.
- */
-pid_t qemu_fork(Error **errp);
-
/* Using intptr_t ensures that qemu_*_page_mask is sign-extended even
* when intptr_t is 32-bit and we are aligning a long long.
*/
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 77d882e681..760390b31e 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -583,76 +583,6 @@ char *qemu_get_pid_name(pid_t pid)
}
-pid_t qemu_fork(Error **errp)
-{
-sigset_t oldmask, newmask;
-struct sigaction sig_action;
-int saved_errno;
-pid_t pid;
-
-/*
- * Need to block signals now, so that child process can safely
- * kill off caller's signal handlers without a race.
- */
-sigfillset(&newmask);
-if (pthread_sigmask(SIG_SETMASK, &newmask, &oldmask) != 0) {
-error_setg_errno(errp, errno,
- "cannot block signals");
-return -1;
-}
-
-pid = fork();
-saved_errno = errno;
-
-if (pid < 0) {
-/* attempt to restore signal mask, but ignore failure, to
- * avoid obscuring the fork failure */
-(void)pthread_sigmask(SIG_SETMASK, &oldmask, NULL);
-error_setg_errno(errp, saved_errno,
- "cannot fork child process");
-errno = saved_errno;
-return -1;
-} else if (pid) {
-/* parent process */
-
-/* Restore our original signal mask now that the child is
- * safely running. Only documented failures are EFAULT (not
- * possible, since we are using just-grabbed mask) or EINVAL
- * (not possible, since we are using correct arguments). */
-(void)pthread_sigmask(SIG_SETMASK, &oldmask, NULL);
-} else {
-/* child process */
-size_t i;
-
-/* Clear out all signal handlers from parent so nothing
- * unexpected can happen in our child once we unblock
- * signals */
-sig_action.sa_handler = SIG_DFL;
-sig_action.sa_flags = 0;
-sigemptyset(&sig_action.sa_mask);
-
-for (i = 1; i < NSIG; i++) {
-/* Only possible errors are EFAULT or EINVAL The former
- * won't happen, the latter we expect, so no need to check
- * return value */
-(void)sigaction(i, &sig_action, NULL);
-}
-
-/* Unmask all signals in child, since we've no idea what the
- * caller's done with their signal mask and don't want to
- * propagate that to children */
-sigemptyset(&newmask);
-if (pthread_sigmask(SIG_SETMASK, &newmask, NULL) != 0) {
-Error *local_err = NULL;
-error_setg_errno(&local_err, errno,
- "cannot unblock signals");
-error_report_err(local_err);
-_exit(1);
-}
-}
-return pid;
-}
-
void *qemu_alloc_stack(size_t *sz)
{
void *ptr, *guardpage;
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 07ade41800..528c9ee156 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -283,15 +283,6 @@ char *qemu_get_pid_name(pid_t pid)
}
-pid_t qemu_fork(Error **errp)
-{
-errno = ENOSYS;
-error_setg_errno(errp, errno,
- "cannot fork child process");
-return -1;
-}
-
-
#undef connect
int qemu_connect_wrap(int sockfd, const struct sockaddr *addr,
socklen_t addrlen)
--
2.39.2
[PULL v2 04/25] tests: add test-error-report
From: Marc-André Lureau
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-5-marcandre.lur...@redhat.com>
---
tests/unit/test-error-report.c | 121 +
tests/unit/meson.build | 1 +
2 files changed, 122 insertions(+)
create mode 100644 tests/unit/test-error-report.c
diff --git a/tests/unit/test-error-report.c b/tests/unit/test-error-report.c
new file mode 100644
index 00..b09650687b
--- /dev/null
+++ b/tests/unit/test-error-report.c
@@ -0,0 +1,121 @@
+/*
+ * Error reporting test
+ *
+ * Copyright (C) 2022 Red Hat Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "glib-compat.h"
+#include
+
+#include "qemu/error-report.h"
+
+static void
+test_error_report_simple(void)
+{
+if (g_test_subprocess()) {
+error_report("%s", "test error");
+warn_report("%s", "test warn");
+info_report("%s", "test info");
+return;
+}
+
+g_test_trap_subprocess(NULL, 0, 0);
+g_test_trap_assert_passed();
+g_test_trap_assert_stderr("\
+test-error-report: test error*\
+test-error-report: warning: test warn*\
+test-error-report: info: test info*\
+");
+}
+
+static void
+test_error_report_loc(void)
+{
+if (g_test_subprocess()) {
+loc_set_file("some-file.c", 7717);
+error_report("%s", "test error1");
+loc_set_none();
+error_report("%s", "test error2");
+return;
+}
+
+g_test_trap_subprocess(NULL, 0, 0);
+g_test_trap_assert_passed();
+g_test_trap_assert_stderr("\
+test-error-report:some-file.c:7717: test error1*\
+test-error-report: test error2*\
+");
+}
+
+static void
+test_error_report_glog(void)
+{
+if (g_test_subprocess()) {
+g_message("gmessage");
+return;
+}
+
+g_test_trap_subprocess(NULL, 0, 0);
+g_test_trap_assert_passed();
+g_test_trap_assert_stderr("test-error-report: info: gmessage*");
+}
+
+static void
+test_error_report_once(void)
+{
+int i;
+
+if (g_test_subprocess()) {
+for (i = 0; i < 3; i++) {
+warn_report_once("warn");
+error_report_once("err");
+}
+return;
+}
+
+g_test_trap_subprocess(NULL, 0, 0);
+g_test_trap_assert_passed();
+g_test_trap_assert_stderr("\
+test-error-report: warning: warn*\
+test-error-report: err*\
+");
+}
+
+static void
+test_error_report_timestamp(void)
+{
+if (g_test_subprocess()) {
+message_with_timestamp = true;
+warn_report("warn");
+error_report("err");
+return;
+}
+
+g_test_trap_subprocess(NULL, 0, 0);
+g_test_trap_assert_passed();
+g_test_trap_assert_stderr("\
+*-*-*:*:* test-error-report: warning: warn*\
+*-*-*:*:* test-error-report: err*\
+");
+}
+
+int
+main(int argc, char *argv[])
+{
+setlocale(LC_ALL, "");
+
+g_test_init(&argc, &argv, NULL);
+error_init("test-error-report");
+
+g_test_add_func("/error-report/simple", test_error_report_simple);
+g_test_add_func("/error-report/loc", test_error_report_loc);
+g_test_add_func("/error-report/glog", test_error_report_glog);
+g_test_add_func("/error-report/once", test_error_report_once);
+g_test_add_func("/error-report/timestamp", test_error_report_timestamp);
+
+return g_test_run();
+}
diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index d9c0a7eae6..fa63cfe6ff 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -11,6 +11,7 @@ tests = {
'check-qobject': [],
'check-qjson': [],
'check-qlit': [],
+ 'test-error-report': [],
'test-qobject-output-visitor': [testqapi],
'test-clone-visitor': [testqapi],
'test-qobject-input-visitor': [testqapi],
--
2.39.2
[PULL 10/25] main-loop: remove qemu_fd_register(), win32/slirp/socket specific
From: Marc-André Lureau
Open-code the socket registration where it's needed, to avoid
artificially used or unclear generic interface.
Furthermore, the following patches are going to make socket handling use
FD-only inside QEMU, but we need to handle win32 SOCKET from libslirp.
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-12-marcandre.lur...@redhat.com>
---
include/qemu/main-loop.h | 2 --
net/slirp.c | 8 +++-
util/main-loop.c | 11 ---
3 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index c25f390696..b3e54e00bc 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -387,8 +387,6 @@ void qemu_cond_timedwait_iothread(QemuCond *cond, int ms);
/* internal interfaces */
-void qemu_fd_register(int fd);
-
#define qemu_bh_new(cb, opaque) \
qemu_bh_new_full((cb), (opaque), (stringify(cb)))
QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb, void *opaque, const char *name);
diff --git a/net/slirp.c b/net/slirp.c
index 2ee3f1a0d7..0730a935ba 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -248,7 +248,13 @@ static void net_slirp_timer_mod(void *timer, int64_t
expire_timer,
static void net_slirp_register_poll_fd(int fd, void *opaque)
{
-qemu_fd_register(fd);
+#ifdef WIN32
+AioContext *ctxt = qemu_get_aio_context();
+
+qemu_socket_select(fd, event_notifier_get_handle(&ctxt->notifier),
+ FD_READ | FD_ACCEPT | FD_CLOSE |
+ FD_CONNECT | FD_WRITE | FD_OOB, NULL);
+#endif
}
static void net_slirp_unregister_poll_fd(int fd, void *opaque)
diff --git a/util/main-loop.c b/util/main-loop.c
index 16e837fb12..e180c85145 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -252,10 +252,6 @@ static int max_priority;
static int glib_pollfds_idx;
static int glib_n_poll_fds;
-void qemu_fd_register(int fd)
-{
-}
-
static void glib_pollfds_fill(int64_t *cur_timeout)
{
GMainContext *context = g_main_context_default();
@@ -414,13 +410,6 @@ void qemu_del_wait_object(HANDLE handle, WaitObjectFunc
*func, void *opaque)
}
}
-void qemu_fd_register(int fd)
-{
-qemu_socket_select(fd,
event_notifier_get_handle(&qemu_aio_context->notifier),
- FD_READ | FD_ACCEPT | FD_CLOSE |
- FD_CONNECT | FD_WRITE | FD_OOB, NULL);
-}
-
static int pollfds_fill(GArray *pollfds, fd_set *rfds, fd_set *wfds,
fd_set *xfds)
{
--
2.39.2
[PULL 02/25] tests: use closesocket()
From: Marc-André Lureau
Because they are actually sockets...
Signed-off-by: Marc-André Lureau
Reviewed-by: Thomas Huth
Message-Id: <20230221124802.4103554-3-marcandre.lur...@redhat.com>
---
tests/unit/socket-helpers.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/unit/socket-helpers.c b/tests/unit/socket-helpers.c
index eecadf3a3c..914b3aa0cf 100644
--- a/tests/unit/socket-helpers.c
+++ b/tests/unit/socket-helpers.c
@@ -117,13 +117,13 @@ static int socket_can_bind_connect(const char *hostname,
int family)
cleanup:
if (afd != -1) {
-close(afd);
+closesocket(afd);
}
if (cfd != -1) {
-close(cfd);
+closesocket(cfd);
}
if (lfd != -1) {
-close(lfd);
+closesocket(lfd);
}
if (res) {
freeaddrinfo(res);
--
2.39.2
[PULL v2 24/25] qtest: enable vnc-display test on win32
From: Marc-André Lureau
Now that qtest_qmp_add_client() works on win32, we can enable the VNC
test.
Signed-off-by: Marc-André Lureau
Acked-by: Thomas Huth
Message-Id: <20230306122751.2355515-11-marcandre.lur...@redhat.com>
---
tests/qtest/vnc-display-test.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/tests/qtest/vnc-display-test.c b/tests/qtest/vnc-display-test.c
index e52a4326ec..f8933b0761 100644
--- a/tests/qtest/vnc-display-test.c
+++ b/tests/qtest/vnc-display-test.c
@@ -19,7 +19,7 @@ typedef struct Test {
GMainLoop *loop;
} Test;
-#if !defined(WIN32) && !defined(CONFIG_DARWIN)
+#if !defined(CONFIG_DARWIN)
static void on_vnc_error(VncConnection* self,
const char* msg)
@@ -38,10 +38,7 @@ static void on_vnc_auth_failure(VncConnection *self,
static bool
test_setup(Test *test)
{
-#ifdef WIN32
-g_test_skip("Not supported on Windows yet");
-return false;
-#elif defined(CONFIG_DARWIN)
+#if defined(CONFIG_DARWIN)
g_test_skip("Broken on Darwin");
return false;
#else
@@ -59,7 +56,12 @@ test_setup(Test *test)
g_signal_connect(test->conn, "vnc-auth-failure",
G_CALLBACK(on_vnc_auth_failure), NULL);
vnc_connection_set_auth_type(test->conn, VNC_CONNECTION_AUTH_NONE);
+
+#ifdef WIN32
+vnc_connection_open_fd(test->conn, _get_osfhandle(pair[0]));
+#else
vnc_connection_open_fd(test->conn, pair[0]);
+#endif
test->loop = g_main_loop_new(NULL, FALSE);
return true;
--
2.39.2
[PULL v2 16/25] tests: fix path separator, use g_build_filename()
From: Marc-André Lureau
Signed-off-by: Marc-André Lureau
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Thomas Huth
Message-Id: <20230306122751.2355515-2-marcandre.lur...@redhat.com>
---
tests/unit/test-io-channel-command.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/unit/test-io-channel-command.c
b/tests/unit/test-io-channel-command.c
index c6e66a8c33..4f022617df 100644
--- a/tests/unit/test-io-channel-command.c
+++ b/tests/unit/test-io-channel-command.c
@@ -35,7 +35,7 @@ static char *socat = NULL;
static void test_io_channel_command_fifo(bool async)
{
g_autofree gchar *tmpdir = g_dir_make_tmp("qemu-test-io-channel.XX",
NULL);
-g_autofree gchar *fifo = g_strdup_printf("%s/%s", tmpdir, TEST_FIFO);
+g_autofree gchar *fifo = g_build_filename(tmpdir, TEST_FIFO, NULL);
g_autofree gchar *srcargs = g_strdup_printf("%s - PIPE:%s,wronly", socat,
fifo);
g_autofree gchar *dstargs = g_strdup_printf("%s PIPE:%s,rdonly -", socat,
fifo);
g_auto(GStrv) srcargv = g_strsplit(srcargs, " ", -1);
--
2.39.2
[PULL v2 15/25] win32: replace closesocket() with close() wrapper
From: Marc-André Lureau
Use a close() wrapper instead, so that we don't need to worry about
closesocket() vs close() anymore, let's hope.
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-17-marcandre.lur...@redhat.com>
---
include/sysemu/os-posix.h | 2 --
include/sysemu/os-win32.h | 8 +++---
backends/tpm/tpm_emulator.c | 6 ++--
crypto/afalg.c | 6 ++--
hw/hyperv/syndbg.c | 4 +--
io/channel-socket.c | 10 +++
net/dgram.c | 14 +-
net/socket.c| 22 +++
tests/qtest/libqtest.c | 8 +++---
tests/qtest/microbit-test.c | 2 +-
tests/qtest/netdev-socket.c | 10 +++
tests/unit/socket-helpers.c | 8 +++---
util/oslib-win32.c | 56 +++--
util/qemu-sockets.c | 22 +++
14 files changed, 89 insertions(+), 89 deletions(-)
diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index 378213fc86..1030d39904 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -51,8 +51,6 @@ void os_daemonize(void);
void os_setup_post(void);
int os_mlock(void);
-#define closesocket(s) close(s)
-
int os_set_daemonize(bool d);
bool is_daemonized(void);
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index cb1dcce618..e2849f88ab 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -175,6 +175,10 @@ bool qemu_socket_unselect(int sockfd, Error **errp);
* set errno based on WSAGetLastError()
*/
+#undef close
+#define close qemu_close_wrap
+int qemu_close_wrap(int fd);
+
#undef connect
#define connect qemu_connect_wrap
int qemu_connect_wrap(int sockfd, const struct sockaddr *addr,
@@ -206,10 +210,6 @@ int qemu_shutdown_wrap(int sockfd, int how);
#define ioctlsocket qemu_ioctlsocket_wrap
int qemu_ioctlsocket_wrap(int fd, int req, void *val);
-#undef closesocket
-#define closesocket qemu_closesocket_wrap
-int qemu_closesocket_wrap(int fd);
-
#undef getsockopt
#define getsockopt qemu_getsockopt_wrap
int qemu_getsockopt_wrap(int sockfd, int level, int optname,
diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
index d18144b92e..402a2d6312 100644
--- a/backends/tpm/tpm_emulator.c
+++ b/backends/tpm/tpm_emulator.c
@@ -573,13 +573,13 @@ static int tpm_emulator_prepare_data_fd(TPMEmulator
*tpm_emu)
goto err_exit;
}
-closesocket(fds[1]);
+close(fds[1]);
return 0;
err_exit:
-closesocket(fds[0]);
-closesocket(fds[1]);
+close(fds[0]);
+close(fds[1]);
return -1;
}
diff --git a/crypto/afalg.c b/crypto/afalg.c
index 10046bb0ae..348301e703 100644
--- a/crypto/afalg.c
+++ b/crypto/afalg.c
@@ -59,7 +59,7 @@ qcrypto_afalg_socket_bind(const char *type, const char *name,
if (bind(sbind, (const struct sockaddr *)&salg, sizeof(salg)) != 0) {
error_setg_errno(errp, errno, "Failed to bind socket");
-closesocket(sbind);
+close(sbind);
return -1;
}
@@ -105,11 +105,11 @@ void qcrypto_afalg_comm_free(QCryptoAFAlg *afalg)
}
if (afalg->tfmfd != -1) {
-closesocket(afalg->tfmfd);
+close(afalg->tfmfd);
}
if (afalg->opfd != -1) {
-closesocket(afalg->opfd);
+close(afalg->opfd);
}
g_free(afalg);
diff --git a/hw/hyperv/syndbg.c b/hw/hyperv/syndbg.c
index 94fe1b534b..065e12fb1e 100644
--- a/hw/hyperv/syndbg.c
+++ b/hw/hyperv/syndbg.c
@@ -340,7 +340,7 @@ static void hv_syndbg_realize(DeviceState *dev, Error
**errp)
syndbg->servaddr.sin_family = AF_INET;
if (connect(syndbg->socket, (struct sockaddr *)&syndbg->servaddr,
sizeof(syndbg->servaddr)) < 0) {
-closesocket(syndbg->socket);
+close(syndbg->socket);
error_setg(errp, "%s failed to connect to socket", TYPE_HV_SYNDBG);
return;
}
@@ -357,7 +357,7 @@ static void hv_syndbg_unrealize(DeviceState *dev)
if (syndbg->socket > 0) {
qemu_set_fd_handler(syndbg->socket, NULL, NULL, NULL);
-closesocket(syndbg->socket);
+close(syndbg->socket);
}
}
diff --git a/io/channel-socket.c b/io/channel-socket.c
index 03757c7a7e..b0ea7d48b3 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -159,7 +159,7 @@ int qio_channel_socket_connect_sync(QIOChannelSocket *ioc,
trace_qio_channel_socket_connect_complete(ioc, fd);
if (qio_channel_socket_set_fd(ioc, fd, errp) < 0) {
-closesocket(fd);
+close(fd);
return -1;
}
@@ -233,7 +233,7 @@ int qio_channel_socket_listen_sync(QIOChannelSocket *ioc,
trace_qio_channel_socket_listen_complete(ioc, fd);
if (qio_channel_socket_set_fd(ioc, fd, errp) < 0) {
-closesocket(fd);
+close(fd);
return -1;
}
qio_channel_set_feature(QIO_CHANNEL(ioc), QIO_CHANNEL_FEATURE_LISTEN);
@@ -310,7 +310,7 @@ int qio_channel_socket_dgram_s
[PULL v2 23/25] libqtest: make qtest_qmp_add_client work on win32
From: Marc-André Lureau
Use the "get-win32-socket" function to pass an opened socket to QEMU,
instead of using "getfd", which relies on socket ancillary FD message
passing.
Signed-off-by: Marc-André Lureau
Reviewed-by: Daniel P. Berrangé
Message-Id: <20230306122751.2355515-10-marcandre.lur...@redhat.com>
---
tests/qtest/libqtest.h | 5 ++---
tests/qtest/libqtest.c | 18 --
2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index fcf1c3c3b3..8d7d450963 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -758,17 +758,16 @@ void qtest_qmp_device_add_qdict(QTestState *qts, const
char *drv,
void qtest_qmp_device_add(QTestState *qts, const char *driver, const char *id,
const char *fmt, ...) G_GNUC_PRINTF(4, 5);
-#ifndef _WIN32
/**
* qtest_qmp_add_client:
* @qts: QTestState instance to operate on
* @protocol: the protocol to add to
* @fd: the client file-descriptor
*
- * Call QMP ``getfd`` followed by ``add_client`` with the given @fd.
+ * Call QMP ``getfd`` (on Windows ``get-win32-socket``) followed by
+ * ``add_client`` with the given @fd.
*/
void qtest_qmp_add_client(QTestState *qts, const char *protocol, int fd);
-#endif /* _WIN32 */
/**
* qtest_qmp_device_del_send:
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index dee2032331..c3a0ef5bb4 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1478,13 +1478,28 @@ void qtest_qmp_device_add(QTestState *qts, const char
*driver, const char *id,
qobject_unref(args);
}
-#ifndef _WIN32
void qtest_qmp_add_client(QTestState *qts, const char *protocol, int fd)
{
QDict *resp;
+#ifdef WIN32
+WSAPROTOCOL_INFOW info;
+g_autofree char *info64 = NULL;
+SOCKET s;
+
+assert(fd_is_socket(fd));
+s = _get_osfhandle(fd);
+if (WSADuplicateSocketW(s, GetProcessId((HANDLE)qts->qemu_pid), &info) ==
SOCKET_ERROR) {
+g_autofree char *emsg = g_win32_error_message(WSAGetLastError());
+g_error("WSADuplicateSocketW failed: %s", emsg);
+}
+info64 = g_base64_encode((guchar *)&info, sizeof(info));
+resp = qtest_qmp(qts, "{'execute': 'get-win32-socket',"
+ "'arguments': {'fdname': 'fdname', 'info': %s}}", info64);
+#else
resp = qtest_qmp_fds(qts, &fd, 1, "{'execute': 'getfd',"
"'arguments': {'fdname': 'fdname'}}");
+#endif
g_assert(resp);
g_assert(!qdict_haskey(resp, "event")); /* We don't expect any events */
g_assert(!qdict_haskey(resp, "error"));
@@ -1498,7 +1513,6 @@ void qtest_qmp_add_client(QTestState *qts, const char
*protocol, int fd)
g_assert(!qdict_haskey(resp, "error"));
qobject_unref(resp);
}
-#endif
/*
* Generic hot-unplugging test via the device_del QMP command.
--
2.39.2
[PULL 19/25] osdep: implement qemu_socketpair() for win32
From: Marc-André Lureau
Manually implement a socketpair() function, using UNIX sockets and
simple peer credential checking.
QEMU doesn't make much use of socketpair, beside vhost-user which is not
available for win32 at this point. However, I intend to use it for
writing some new portable tests.
Signed-off-by: Marc-André Lureau
Reviewed-by: Daniel P. Berrangé
Message-Id: <20230306122751.2355515-5-marcandre.lur...@redhat.com>
---
include/qemu/sockets.h | 2 -
util/oslib-win32.c | 110 +
2 files changed, 110 insertions(+), 2 deletions(-)
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 2b0698a7c9..d935fd80da 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -15,7 +15,6 @@ int inet_aton(const char *cp, struct in_addr *ia);
bool fd_is_socket(int fd);
int qemu_socket(int domain, int type, int protocol);
-#ifndef WIN32
/**
* qemu_socketpair:
* @domain: specifies a communication domain, such as PF_UNIX
@@ -30,7 +29,6 @@ int qemu_socket(int domain, int type, int protocol);
* Return 0 on success.
*/
int qemu_socketpair(int domain, int type, int protocol, int sv[2]);
-#endif
int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);
/*
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 29a667ae3d..16f8a67f7e 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -310,6 +310,116 @@ bool qemu_socket_unselect(int sockfd, Error **errp)
return qemu_socket_select(sockfd, NULL, 0, errp);
}
+int qemu_socketpair(int domain, int type, int protocol, int sv[2])
+{
+struct sockaddr_un addr = {
+0,
+};
+socklen_t socklen;
+int listener = -1;
+int client = -1;
+int server = -1;
+g_autofree char *path = NULL;
+int tmpfd;
+u_long arg;
+int ret = -1;
+
+g_return_val_if_fail(sv != NULL, -1);
+
+addr.sun_family = AF_UNIX;
+socklen = sizeof(addr);
+
+tmpfd = g_file_open_tmp(NULL, &path, NULL);
+if (tmpfd == -1 || !path) {
+errno = EACCES;
+goto out;
+}
+
+close(tmpfd);
+
+if (strlen(path) >= sizeof(addr.sun_path)) {
+errno = EINVAL;
+goto out;
+}
+
+strncpy(addr.sun_path, path, sizeof(addr.sun_path) - 1);
+
+listener = socket(domain, type, protocol);
+if (listener == -1) {
+goto out;
+}
+
+if (DeleteFile(path) == 0 && GetLastError() != ERROR_FILE_NOT_FOUND) {
+errno = EACCES;
+goto out;
+}
+g_clear_pointer(&path, g_free);
+
+if (bind(listener, (struct sockaddr *)&addr, socklen) == -1) {
+goto out;
+}
+
+if (listen(listener, 1) == -1) {
+goto out;
+}
+
+client = socket(domain, type, protocol);
+if (client == -1) {
+goto out;
+}
+
+arg = 1;
+if (ioctlsocket(client, FIONBIO, &arg) != NO_ERROR) {
+goto out;
+}
+
+if (connect(client, (struct sockaddr *)&addr, socklen) == -1 &&
+WSAGetLastError() != WSAEWOULDBLOCK) {
+goto out;
+}
+
+server = accept(listener, NULL, NULL);
+if (server == -1) {
+goto out;
+}
+
+arg = 0;
+if (ioctlsocket(client, FIONBIO, &arg) != NO_ERROR) {
+goto out;
+}
+
+arg = 0;
+if (ioctlsocket(client, SIO_AF_UNIX_GETPEERPID, &arg) != NO_ERROR) {
+goto out;
+}
+
+if (arg != GetCurrentProcessId()) {
+errno = EPERM;
+goto out;
+}
+
+sv[0] = server;
+server = -1;
+sv[1] = client;
+client = -1;
+ret = 0;
+
+out:
+if (listener != -1) {
+close(listener);
+}
+if (client != -1) {
+close(client);
+}
+if (server != -1) {
+close(server);
+}
+if (path) {
+DeleteFile(path);
+}
+return ret;
+}
+
#undef connect
int qemu_connect_wrap(int sockfd, const struct sockaddr *addr,
socklen_t addrlen)
--
2.39.2
[PULL v2 13/25] win32: avoid mixing SOCKET and file descriptor space
From: Marc-André Lureau
Until now, a win32 SOCKET handle is often cast to an int file
descriptor, as this is what other OS use for sockets. When necessary,
QEMU eventually queries whether it's a socket with the help of
fd_is_socket(). However, there is no guarantee of conflict between the
fd and SOCKET space. Such conflict would have surprising consequences,
we shouldn't mix them.
Also, it is often forgotten that SOCKET must be closed with
closesocket(), and not close().
Instead, let's make the win32 socket wrapper functions return and take a
file descriptor, and let util/ wrappers do the fd/SOCKET conversion as
necessary. A bit of adaptation is necessary in io/ as well.
Unfortunately, we can't drop closesocket() usage, despite
_open_osfhandle() documentation claiming transfer of ownership, testing
shows bad behaviour if you forget to call closesocket().
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-15-marcandre.lur...@redhat.com>
---
include/sysemu/os-win32.h | 4 +-
io/channel-watch.c| 6 +-
util/aio-win32.c | 9 +-
util/oslib-win32.c| 219 --
4 files changed, 197 insertions(+), 41 deletions(-)
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 504a8966c3..cb1dcce618 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -166,10 +166,10 @@ static inline void qemu_funlockfile(FILE *f)
}
/* Helper for WSAEventSelect, to report errors */
-bool qemu_socket_select(SOCKET s, WSAEVENT hEventObject,
+bool qemu_socket_select(int sockfd, WSAEVENT hEventObject,
long lNetworkEvents, Error **errp);
-bool qemu_socket_unselect(SOCKET s, Error **errp);
+bool qemu_socket_unselect(int sockfd, Error **errp);
/* We wrap all the sockets functions so that we can
* set errno based on WSAGetLastError()
diff --git a/io/channel-watch.c b/io/channel-watch.c
index 6ac41009fa..64b486e378 100644
--- a/io/channel-watch.c
+++ b/io/channel-watch.c
@@ -275,13 +275,13 @@ GSource *qio_channel_create_fd_watch(QIOChannel *ioc,
#ifdef CONFIG_WIN32
GSource *qio_channel_create_socket_watch(QIOChannel *ioc,
- int socket,
+ int sockfd,
GIOCondition condition)
{
GSource *source;
QIOChannelSocketSource *ssource;
-qemu_socket_select(socket, ioc->event,
+qemu_socket_select(sockfd, ioc->event,
FD_READ | FD_ACCEPT | FD_CLOSE |
FD_CONNECT | FD_WRITE | FD_OOB, NULL);
@@ -293,7 +293,7 @@ GSource *qio_channel_create_socket_watch(QIOChannel *ioc,
object_ref(OBJECT(ioc));
ssource->condition = condition;
-ssource->socket = socket;
+ssource->socket = _get_osfhandle(sockfd);
ssource->revents = 0;
ssource->fd.fd = (gintptr)ioc->event;
diff --git a/util/aio-win32.c b/util/aio-win32.c
index 08e8f5615d..6bded009a4 100644
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -73,15 +73,18 @@ void aio_set_fd_handler(AioContext *ctx,
{
AioHandler *old_node;
AioHandler *node = NULL;
+SOCKET s;
if (!fd_is_socket(fd)) {
error_report("fd=%d is not a socket, AIO implementation is missing",
fd);
return;
}
+s = _get_osfhandle(fd);
+
qemu_lockcnt_lock(&ctx->list_lock);
QLIST_FOREACH(old_node, &ctx->aio_handlers, node) {
-if (old_node->pfd.fd == fd && !old_node->deleted) {
+if (old_node->pfd.fd == s && !old_node->deleted) {
break;
}
}
@@ -92,7 +95,7 @@ void aio_set_fd_handler(AioContext *ctx,
/* Alloc and insert if it's not already there */
node = g_new0(AioHandler, 1);
-node->pfd.fd = fd;
+node->pfd.fd = s;
node->pfd.events = 0;
if (node->io_read) {
@@ -120,7 +123,7 @@ void aio_set_fd_handler(AioContext *ctx,
QLIST_INSERT_HEAD_RCU(&ctx->aio_handlers, node, node);
event = event_notifier_get_handle(&ctx->notifier);
-qemu_socket_select(node->pfd.fd, event, bitmask, NULL);
+qemu_socket_select(fd, event, bitmask, NULL);
}
if (old_node) {
aio_remove_fd_handler(ctx, old_node);
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index dbd32acc98..7836fb0be3 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -283,13 +283,20 @@ char *qemu_get_pid_name(pid_t pid)
}
-bool qemu_socket_select(SOCKET s, WSAEVENT hEventObject,
+bool qemu_socket_select(int sockfd, WSAEVENT hEventObject,
long lNetworkEvents, Error **errp)
{
+SOCKET s = _get_osfhandle(sockfd);
+
if (errp == NULL) {
errp = &error_warn;
}
+if (s == INVALID_SOCKET) {
+error_setg(errp, "invalid socket fd=%d", sockfd);
+return false;
+}
+
if (WSAEventSelect(s, hEventObject, lNetworkEvents) != 0) {
e
[PULL 20/25] qmp: 'add_client' actually expects sockets
From: Marc-André Lureau
Whether it is SPICE, VNC, D-Bus, or the socket chardev, they all
actually expect a socket kind or will fail in different ways at runtime.
Throw an error early if the given 'add_client' fd is not a socket, and
close it to avoid leaks.
This allows to replace the close() call with a more correct & portable
closesocket() version.
(this will allow importing sockets on Windows with a specialized command
in the following patch, while keeping the remaining monitor associated
sockets/add_client code & usage untouched)
Signed-off-by: Marc-André Lureau
Reviewed-by: Philippe Mathieu-Daudé
Acked-by: Markus Armbruster
Message-Id: <20230306122751.2355515-6-marcandre.lur...@redhat.com>
---
qapi/misc.json | 3 +++
monitor/qmp-cmds.c | 7 +++
2 files changed, 10 insertions(+)
diff --git a/qapi/misc.json b/qapi/misc.json
index 27ef5a2b20..f0217cfba0 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -14,6 +14,9 @@
# Allow client connections for VNC, Spice and socket based
# character devices to be passed in to QEMU via SCM_RIGHTS.
#
+# If the FD associated with @fdname is not a socket, the command will fail and
+# the FD will be closed.
+#
# @protocol: protocol name. Valid names are "vnc", "spice", "@dbus-display" or
#the name of a character device (eg. from -chardev id=)
#
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 859012aef4..b0f948d337 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -14,6 +14,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/sockets.h"
#include "monitor-internal.h"
#include "monitor/qdev.h"
#include "monitor/qmp-helpers.h"
@@ -139,6 +140,12 @@ void qmp_add_client(const char *protocol, const char
*fdname,
return;
}
+if (!fd_is_socket(fd)) {
+error_setg(errp, "parameter @fdname must name a socket");
+close(fd);
+return;
+}
+
for (i = 0; i < ARRAY_SIZE(protocol_table); i++) {
if (!strcmp(protocol, protocol_table[i].name)) {
if (!protocol_table[i].add_client(fd, has_skipauth, skipauth,
--
2.39.2
[PULL v2 03/25] io: use closesocket()
From: Marc-André Lureau
Because they are actually sockets...
Signed-off-by: Marc-André Lureau
Reviewed-by: Thomas Huth
Message-Id: <20230221124802.4103554-4-marcandre.lur...@redhat.com>
---
io/channel-socket.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/io/channel-socket.c b/io/channel-socket.c
index 7aca84f61a..2040297d2b 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -159,7 +159,7 @@ int qio_channel_socket_connect_sync(QIOChannelSocket *ioc,
trace_qio_channel_socket_connect_complete(ioc, fd);
if (qio_channel_socket_set_fd(ioc, fd, errp) < 0) {
-close(fd);
+closesocket(fd);
return -1;
}
@@ -233,7 +233,7 @@ int qio_channel_socket_listen_sync(QIOChannelSocket *ioc,
trace_qio_channel_socket_listen_complete(ioc, fd);
if (qio_channel_socket_set_fd(ioc, fd, errp) < 0) {
-close(fd);
+closesocket(fd);
return -1;
}
qio_channel_set_feature(QIO_CHANNEL(ioc), QIO_CHANNEL_FEATURE_LISTEN);
@@ -310,7 +310,7 @@ int qio_channel_socket_dgram_sync(QIOChannelSocket *ioc,
trace_qio_channel_socket_dgram_complete(ioc, fd);
if (qio_channel_socket_set_fd(ioc, fd, errp) < 0) {
-close(fd);
+closesocket(fd);
return -1;
}
--
2.39.2
[PULL v2 10/25] main-loop: remove qemu_fd_register(), win32/slirp/socket specific
From: Marc-André Lureau
Open-code the socket registration where it's needed, to avoid
artificially used or unclear generic interface.
Furthermore, the following patches are going to make socket handling use
FD-only inside QEMU, but we need to handle win32 SOCKET from libslirp.
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-12-marcandre.lur...@redhat.com>
---
include/qemu/main-loop.h | 2 --
net/slirp.c | 8 +++-
util/main-loop.c | 11 ---
3 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index c25f390696..b3e54e00bc 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -387,8 +387,6 @@ void qemu_cond_timedwait_iothread(QemuCond *cond, int ms);
/* internal interfaces */
-void qemu_fd_register(int fd);
-
#define qemu_bh_new(cb, opaque) \
qemu_bh_new_full((cb), (opaque), (stringify(cb)))
QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb, void *opaque, const char *name);
diff --git a/net/slirp.c b/net/slirp.c
index 2ee3f1a0d7..0730a935ba 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -248,7 +248,13 @@ static void net_slirp_timer_mod(void *timer, int64_t
expire_timer,
static void net_slirp_register_poll_fd(int fd, void *opaque)
{
-qemu_fd_register(fd);
+#ifdef WIN32
+AioContext *ctxt = qemu_get_aio_context();
+
+qemu_socket_select(fd, event_notifier_get_handle(&ctxt->notifier),
+ FD_READ | FD_ACCEPT | FD_CLOSE |
+ FD_CONNECT | FD_WRITE | FD_OOB, NULL);
+#endif
}
static void net_slirp_unregister_poll_fd(int fd, void *opaque)
diff --git a/util/main-loop.c b/util/main-loop.c
index 16e837fb12..e180c85145 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -252,10 +252,6 @@ static int max_priority;
static int glib_pollfds_idx;
static int glib_n_poll_fds;
-void qemu_fd_register(int fd)
-{
-}
-
static void glib_pollfds_fill(int64_t *cur_timeout)
{
GMainContext *context = g_main_context_default();
@@ -414,13 +410,6 @@ void qemu_del_wait_object(HANDLE handle, WaitObjectFunc
*func, void *opaque)
}
}
-void qemu_fd_register(int fd)
-{
-qemu_socket_select(fd,
event_notifier_get_handle(&qemu_aio_context->notifier),
- FD_READ | FD_ACCEPT | FD_CLOSE |
- FD_CONNECT | FD_WRITE | FD_OOB, NULL);
-}
-
static int pollfds_fill(GArray *pollfds, fd_set *rfds, fd_set *wfds,
fd_set *xfds)
{
--
2.39.2
[PULL 15/25] win32: replace closesocket() with close() wrapper
From: Marc-André Lureau
Use a close() wrapper instead, so that we don't need to worry about
closesocket() vs close() anymore, let's hope.
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-17-marcandre.lur...@redhat.com>
---
include/sysemu/os-posix.h | 2 --
include/sysemu/os-win32.h | 8 +++---
backends/tpm/tpm_emulator.c | 6 ++--
crypto/afalg.c | 6 ++--
hw/hyperv/syndbg.c | 4 +--
io/channel-socket.c | 10 +++
net/dgram.c | 14 +-
net/socket.c| 22 +++
tests/qtest/libqtest.c | 8 +++---
tests/qtest/microbit-test.c | 2 +-
tests/qtest/netdev-socket.c | 10 +++
tests/unit/socket-helpers.c | 8 +++---
util/oslib-win32.c | 56 +++--
util/qemu-sockets.c | 22 +++
14 files changed, 89 insertions(+), 89 deletions(-)
diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index 378213fc86..1030d39904 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -51,8 +51,6 @@ void os_daemonize(void);
void os_setup_post(void);
int os_mlock(void);
-#define closesocket(s) close(s)
-
int os_set_daemonize(bool d);
bool is_daemonized(void);
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index cb1dcce618..e2849f88ab 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -175,6 +175,10 @@ bool qemu_socket_unselect(int sockfd, Error **errp);
* set errno based on WSAGetLastError()
*/
+#undef close
+#define close qemu_close_wrap
+int qemu_close_wrap(int fd);
+
#undef connect
#define connect qemu_connect_wrap
int qemu_connect_wrap(int sockfd, const struct sockaddr *addr,
@@ -206,10 +210,6 @@ int qemu_shutdown_wrap(int sockfd, int how);
#define ioctlsocket qemu_ioctlsocket_wrap
int qemu_ioctlsocket_wrap(int fd, int req, void *val);
-#undef closesocket
-#define closesocket qemu_closesocket_wrap
-int qemu_closesocket_wrap(int fd);
-
#undef getsockopt
#define getsockopt qemu_getsockopt_wrap
int qemu_getsockopt_wrap(int sockfd, int level, int optname,
diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
index d18144b92e..402a2d6312 100644
--- a/backends/tpm/tpm_emulator.c
+++ b/backends/tpm/tpm_emulator.c
@@ -573,13 +573,13 @@ static int tpm_emulator_prepare_data_fd(TPMEmulator
*tpm_emu)
goto err_exit;
}
-closesocket(fds[1]);
+close(fds[1]);
return 0;
err_exit:
-closesocket(fds[0]);
-closesocket(fds[1]);
+close(fds[0]);
+close(fds[1]);
return -1;
}
diff --git a/crypto/afalg.c b/crypto/afalg.c
index 10046bb0ae..348301e703 100644
--- a/crypto/afalg.c
+++ b/crypto/afalg.c
@@ -59,7 +59,7 @@ qcrypto_afalg_socket_bind(const char *type, const char *name,
if (bind(sbind, (const struct sockaddr *)&salg, sizeof(salg)) != 0) {
error_setg_errno(errp, errno, "Failed to bind socket");
-closesocket(sbind);
+close(sbind);
return -1;
}
@@ -105,11 +105,11 @@ void qcrypto_afalg_comm_free(QCryptoAFAlg *afalg)
}
if (afalg->tfmfd != -1) {
-closesocket(afalg->tfmfd);
+close(afalg->tfmfd);
}
if (afalg->opfd != -1) {
-closesocket(afalg->opfd);
+close(afalg->opfd);
}
g_free(afalg);
diff --git a/hw/hyperv/syndbg.c b/hw/hyperv/syndbg.c
index 94fe1b534b..065e12fb1e 100644
--- a/hw/hyperv/syndbg.c
+++ b/hw/hyperv/syndbg.c
@@ -340,7 +340,7 @@ static void hv_syndbg_realize(DeviceState *dev, Error
**errp)
syndbg->servaddr.sin_family = AF_INET;
if (connect(syndbg->socket, (struct sockaddr *)&syndbg->servaddr,
sizeof(syndbg->servaddr)) < 0) {
-closesocket(syndbg->socket);
+close(syndbg->socket);
error_setg(errp, "%s failed to connect to socket", TYPE_HV_SYNDBG);
return;
}
@@ -357,7 +357,7 @@ static void hv_syndbg_unrealize(DeviceState *dev)
if (syndbg->socket > 0) {
qemu_set_fd_handler(syndbg->socket, NULL, NULL, NULL);
-closesocket(syndbg->socket);
+close(syndbg->socket);
}
}
diff --git a/io/channel-socket.c b/io/channel-socket.c
index 03757c7a7e..b0ea7d48b3 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -159,7 +159,7 @@ int qio_channel_socket_connect_sync(QIOChannelSocket *ioc,
trace_qio_channel_socket_connect_complete(ioc, fd);
if (qio_channel_socket_set_fd(ioc, fd, errp) < 0) {
-closesocket(fd);
+close(fd);
return -1;
}
@@ -233,7 +233,7 @@ int qio_channel_socket_listen_sync(QIOChannelSocket *ioc,
trace_qio_channel_socket_listen_complete(ioc, fd);
if (qio_channel_socket_set_fd(ioc, fd, errp) < 0) {
-closesocket(fd);
+close(fd);
return -1;
}
qio_channel_set_feature(QIO_CHANNEL(ioc), QIO_CHANNEL_FEATURE_LISTEN);
@@ -310,7 +310,7 @@ int qio_channel_socket_dgram_s
[PULL 12/25] slirp: open-code qemu_socket_(un)select()
From: Marc-André Lureau
We are about to make the QEMU socket API use file-descriptor space only,
but libslirp gives us SOCKET as fd, still.
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-14-marcandre.lur...@redhat.com>
---
net/slirp.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/net/slirp.c b/net/slirp.c
index a7c35778a6..c33b3e02e7 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -251,16 +251,20 @@ static void net_slirp_register_poll_fd(int fd, void
*opaque)
#ifdef WIN32
AioContext *ctxt = qemu_get_aio_context();
-qemu_socket_select(fd, event_notifier_get_handle(&ctxt->notifier),
+if (WSAEventSelect(fd, event_notifier_get_handle(&ctxt->notifier),
FD_READ | FD_ACCEPT | FD_CLOSE |
- FD_CONNECT | FD_WRITE | FD_OOB, NULL);
+ FD_CONNECT | FD_WRITE | FD_OOB) != 0) {
+error_setg_win32(&error_warn, WSAGetLastError(), "failed to
WSAEventSelect()");
+}
#endif
}
static void net_slirp_unregister_poll_fd(int fd, void *opaque)
{
#ifdef WIN32
-qemu_socket_unselect(fd, NULL);
+if (WSAEventSelect(fd, NULL, 0) != 0) {
+error_setg_win32(&error_warn, WSAGetLastError(), "failed to
WSAEventSelect()");
+}
#endif
}
--
2.39.2
[PULL 23/25] libqtest: make qtest_qmp_add_client work on win32
From: Marc-André Lureau
Use the "get-win32-socket" function to pass an opened socket to QEMU,
instead of using "getfd", which relies on socket ancillary FD message
passing.
Signed-off-by: Marc-André Lureau
Reviewed-by: Daniel P. Berrangé
Message-Id: <20230306122751.2355515-10-marcandre.lur...@redhat.com>
---
tests/qtest/libqtest.h | 5 ++---
tests/qtest/libqtest.c | 18 --
2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index fcf1c3c3b3..8d7d450963 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -758,17 +758,16 @@ void qtest_qmp_device_add_qdict(QTestState *qts, const
char *drv,
void qtest_qmp_device_add(QTestState *qts, const char *driver, const char *id,
const char *fmt, ...) G_GNUC_PRINTF(4, 5);
-#ifndef _WIN32
/**
* qtest_qmp_add_client:
* @qts: QTestState instance to operate on
* @protocol: the protocol to add to
* @fd: the client file-descriptor
*
- * Call QMP ``getfd`` followed by ``add_client`` with the given @fd.
+ * Call QMP ``getfd`` (on Windows ``get-win32-socket``) followed by
+ * ``add_client`` with the given @fd.
*/
void qtest_qmp_add_client(QTestState *qts, const char *protocol, int fd);
-#endif /* _WIN32 */
/**
* qtest_qmp_device_del_send:
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index dee2032331..c3a0ef5bb4 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1478,13 +1478,28 @@ void qtest_qmp_device_add(QTestState *qts, const char
*driver, const char *id,
qobject_unref(args);
}
-#ifndef _WIN32
void qtest_qmp_add_client(QTestState *qts, const char *protocol, int fd)
{
QDict *resp;
+#ifdef WIN32
+WSAPROTOCOL_INFOW info;
+g_autofree char *info64 = NULL;
+SOCKET s;
+
+assert(fd_is_socket(fd));
+s = _get_osfhandle(fd);
+if (WSADuplicateSocketW(s, GetProcessId((HANDLE)qts->qemu_pid), &info) ==
SOCKET_ERROR) {
+g_autofree char *emsg = g_win32_error_message(WSAGetLastError());
+g_error("WSADuplicateSocketW failed: %s", emsg);
+}
+info64 = g_base64_encode((guchar *)&info, sizeof(info));
+resp = qtest_qmp(qts, "{'execute': 'get-win32-socket',"
+ "'arguments': {'fdname': 'fdname', 'info': %s}}", info64);
+#else
resp = qtest_qmp_fds(qts, &fd, 1, "{'execute': 'getfd',"
"'arguments': {'fdname': 'fdname'}}");
+#endif
g_assert(resp);
g_assert(!qdict_haskey(resp, "event")); /* We don't expect any events */
g_assert(!qdict_haskey(resp, "error"));
@@ -1498,7 +1513,6 @@ void qtest_qmp_add_client(QTestState *qts, const char
*protocol, int fd)
g_assert(!qdict_haskey(resp, "error"));
qobject_unref(resp);
}
-#endif
/*
* Generic hot-unplugging test via the device_del QMP command.
--
2.39.2
[PULL 14/25] os-posix: remove useless ioctlsocket() define
From: Marc-André Lureau The API is specific to win32. Signed-off-by: Marc-André Lureau Reviewed-by: Daniel P. Berrangé Message-Id: <20230221124802.4103554-16-marcandre.lur...@redhat.com> --- include/sysemu/os-posix.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h index 58de7c994d..378213fc86 100644 --- a/include/sysemu/os-posix.h +++ b/include/sysemu/os-posix.h @@ -52,7 +52,6 @@ void os_setup_post(void); int os_mlock(void); #define closesocket(s) close(s) -#define ioctlsocket(s, r, v) ioctl(s, r, v) int os_set_daemonize(bool d); bool is_daemonized(void); -- 2.39.2
[PULL 11/25] slirp: unregister the win32 SOCKET
From: Marc-André Lureau
Presumably, this is what should happen when the SOCKET is to be removed.
(it probably worked until now because closesocket() does it implicitly,
but we never now how the slirp library could use the SOCKET later)
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-13-marcandre.lur...@redhat.com>
---
net/slirp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/slirp.c b/net/slirp.c
index 0730a935ba..a7c35778a6 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -259,7 +259,9 @@ static void net_slirp_register_poll_fd(int fd, void *opaque)
static void net_slirp_unregister_poll_fd(int fd, void *opaque)
{
-/* no qemu_fd_unregister */
+#ifdef WIN32
+qemu_socket_unselect(fd, NULL);
+#endif
}
static void net_slirp_notify(void *opaque)
--
2.39.2
[PULL 07/25] win32/socket: introduce qemu_socket_unselect() helper
From: Marc-André Lureau
A more explicit version of qemu_socket_select() with no events.
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-8-marcandre.lur...@redhat.com>
---
include/sysemu/os-win32.h | 2 ++
io/channel-socket.c | 4 ++--
util/oslib-win32.c| 7 ++-
3 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 9f842ae643..504a8966c3 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -169,6 +169,8 @@ static inline void qemu_funlockfile(FILE *f)
bool qemu_socket_select(SOCKET s, WSAEVENT hEventObject,
long lNetworkEvents, Error **errp);
+bool qemu_socket_unselect(SOCKET s, Error **errp);
+
/* We wrap all the sockets functions so that we can
* set errno based on WSAGetLastError()
*/
diff --git a/io/channel-socket.c b/io/channel-socket.c
index 0bc29c4808..03757c7a7e 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -442,7 +442,7 @@ static void qio_channel_socket_finalize(Object *obj)
}
}
#ifdef WIN32
-qemu_socket_select(ioc->fd, NULL, 0, NULL);
+qemu_socket_unselect(ioc->fd, NULL);
#endif
closesocket(ioc->fd);
ioc->fd = -1;
@@ -846,7 +846,7 @@ qio_channel_socket_close(QIOChannel *ioc,
if (sioc->fd != -1) {
#ifdef WIN32
-qemu_socket_select(sioc->fd, NULL, 0, NULL);
+qemu_socket_unselect(sioc->fd, NULL);
#endif
if (qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_LISTEN)) {
socket_listen_cleanup(sioc->fd, errp);
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index df752fc762..dbd32acc98 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -180,7 +180,7 @@ static int socket_error(void)
void qemu_socket_set_block(int fd)
{
unsigned long opt = 0;
-qemu_socket_select(fd, NULL, 0, NULL);
+qemu_socket_unselect(fd, NULL);
ioctlsocket(fd, FIONBIO, &opt);
}
@@ -298,6 +298,11 @@ bool qemu_socket_select(SOCKET s, WSAEVENT hEventObject,
return true;
}
+bool qemu_socket_unselect(SOCKET s, Error **errp)
+{
+return qemu_socket_select(s, NULL, 0, errp);
+}
+
#undef connect
int qemu_connect_wrap(int sockfd, const struct sockaddr *addr,
socklen_t addrlen)
--
2.39.2
[PULL 08/25] aio: make aio_set_fd_poll() static to aio-posix.c
From: Marc-André Lureau
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-9-marcandre.lur...@redhat.com>
---
include/block/aio.h | 8
util/aio-posix.c| 6 +++---
util/aio-win32.c| 7 ---
3 files changed, 3 insertions(+), 18 deletions(-)
diff --git a/include/block/aio.h b/include/block/aio.h
index 8fba6a3584..543717f294 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -482,14 +482,6 @@ void aio_set_fd_handler(AioContext *ctx,
IOHandler *io_poll_ready,
void *opaque);
-/* Set polling begin/end callbacks for a file descriptor that has already been
- * registered with aio_set_fd_handler. Do nothing if the file descriptor is
- * not registered.
- */
-void aio_set_fd_poll(AioContext *ctx, int fd,
- IOHandler *io_poll_begin,
- IOHandler *io_poll_end);
-
/* Register an event notifier and associated callbacks. Behaves very similarly
* to event_notifier_set_handler. Unlike event_notifier_set_handler, these
callbacks
* will be invoked when using aio_poll().
diff --git a/util/aio-posix.c b/util/aio-posix.c
index 6cc6256d53..a8be940f76 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -180,9 +180,9 @@ void aio_set_fd_handler(AioContext *ctx,
}
}
-void aio_set_fd_poll(AioContext *ctx, int fd,
- IOHandler *io_poll_begin,
- IOHandler *io_poll_end)
+static void aio_set_fd_poll(AioContext *ctx, int fd,
+IOHandler *io_poll_begin,
+IOHandler *io_poll_end)
{
AioHandler *node = find_aio_handler(ctx, fd);
diff --git a/util/aio-win32.c b/util/aio-win32.c
index be5136e486..74d63fa21e 100644
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -125,13 +125,6 @@ void aio_set_fd_handler(AioContext *ctx,
aio_notify(ctx);
}
-void aio_set_fd_poll(AioContext *ctx, int fd,
- IOHandler *io_poll_begin,
- IOHandler *io_poll_end)
-{
-/* Not implemented */
-}
-
void aio_set_event_notifier(AioContext *ctx,
EventNotifier *e,
bool is_external,
--
2.39.2
[PULL 16/25] tests: fix path separator, use g_build_filename()
From: Marc-André Lureau
Signed-off-by: Marc-André Lureau
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Thomas Huth
Message-Id: <20230306122751.2355515-2-marcandre.lur...@redhat.com>
---
tests/unit/test-io-channel-command.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/unit/test-io-channel-command.c
b/tests/unit/test-io-channel-command.c
index c6e66a8c33..4f022617df 100644
--- a/tests/unit/test-io-channel-command.c
+++ b/tests/unit/test-io-channel-command.c
@@ -35,7 +35,7 @@ static char *socat = NULL;
static void test_io_channel_command_fifo(bool async)
{
g_autofree gchar *tmpdir = g_dir_make_tmp("qemu-test-io-channel.XX",
NULL);
-g_autofree gchar *fifo = g_strdup_printf("%s/%s", tmpdir, TEST_FIFO);
+g_autofree gchar *fifo = g_build_filename(tmpdir, TEST_FIFO, NULL);
g_autofree gchar *srcargs = g_strdup_printf("%s - PIPE:%s,wronly", socat,
fifo);
g_autofree gchar *dstargs = g_strdup_printf("%s PIPE:%s,rdonly -", socat,
fifo);
g_auto(GStrv) srcargv = g_strsplit(srcargs, " ", -1);
--
2.39.2
[PULL 21/25] monitor: release the lock before calling close()
From: Marc-André Lureau
As per comment, presumably to avoid syscall in critical section.
Fixes: 0210c3b39bef08 ("monitor: Use LOCK_GUARD macros")
Signed-off-by: Marc-André Lureau
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <20230306122751.2355515-7-marcandre.lur...@redhat.com>
---
monitor/fds.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/monitor/fds.c b/monitor/fds.c
index 26b39a0ce6..7daf1064e1 100644
--- a/monitor/fds.c
+++ b/monitor/fds.c
@@ -80,7 +80,8 @@ void qmp_getfd(const char *fdname, Error **errp)
return;
}
-QEMU_LOCK_GUARD(&cur_mon->mon_lock);
+/* See close() call below. */
+qemu_mutex_lock(&cur_mon->mon_lock);
QLIST_FOREACH(monfd, &cur_mon->fds, next) {
if (strcmp(monfd->name, fdname) != 0) {
continue;
@@ -88,6 +89,7 @@ void qmp_getfd(const char *fdname, Error **errp)
tmp_fd = monfd->fd;
monfd->fd = fd;
+qemu_mutex_unlock(&cur_mon->mon_lock);
/* Make sure close() is outside critical section */
close(tmp_fd);
return;
@@ -98,6 +100,7 @@ void qmp_getfd(const char *fdname, Error **errp)
monfd->fd = fd;
QLIST_INSERT_HEAD(&cur_mon->fds, monfd, next);
+qemu_mutex_unlock(&cur_mon->mon_lock);
}
void qmp_closefd(const char *fdname, Error **errp)
--
2.39.2
Re: [PULL 00/25] Win socket patches
Hi On Mon, Mar 13, 2023 at 3:43 PM wrote: > From: Marc-André Lureau > > The following changes since commit > 29c8a9e31a982874ce4e2c15f2bf82d5f8dc3517: > > Merge tag 'linux-user-for-8.0-pull-request' of > https://gitlab.com/laurent_vivier/qemu into staging (2023-03-12 10:57:00 > +) > > are available in the Git repository at: > > https://gitlab.com/marcandre.lureau/qemu.git > tags/win-socket-pull-request > > for you to fetch changes up to 0006c18362cbe1e93587ac1e8ee965c08e6970f2: > > QMP/HMP: only actually implement getfd on CONFIG_POSIX (2023-03-13 > 15:41:32 +0400) > > > QMP command to import win32 sockets > > > > Marc-André Lureau (25): > util: drop qemu_fork() > tests: use closesocket() > io: use closesocket() > tests: add test-error-report > error: add global &error_warn destination > win32/socket: introduce qemu_socket_select() helper > win32/socket: introduce qemu_socket_unselect() helper > aio: make aio_set_fd_poll() static to aio-posix.c > aio/win32: aio_set_fd_handler() only supports SOCKET > main-loop: remove qemu_fd_register(), win32/slirp/socket specific > slirp: unregister the win32 SOCKET > slirp: open-code qemu_socket_(un)select() > win32: avoid mixing SOCKET and file descriptor space > os-posix: remove useless ioctlsocket() define > win32: replace closesocket() with close() wrapper > tests: fix path separator, use g_build_filename() > char: do not double-close fd when failing to add client > tests/docker: fix a win32 error due to portability > osdep: implement qemu_socketpair() for win32 > qmp: 'add_client' actually expects sockets > monitor: release the lock before calling close() > qmp: add 'get-win32-socket' > libqtest: make qtest_qmp_add_client work on win32 > qtest: enable vnc-display test on win32 > QMP/HMP: only actually implement getfd on CONFIG_POSIX > My bad, last patch is not the one I intended (with message fix suggested by Markus). Sending v2. > > qapi/misc.json | 36 ++- > include/block/aio.h | 8 - > include/qapi/error.h | 6 + > include/qemu/main-loop.h | 2 - > include/qemu/osdep.h | 14 -- > include/qemu/sockets.h | 2 - > include/sysemu/os-posix.h| 3 - > include/sysemu/os-win32.h| 15 +- > tests/qtest/libqtest.h | 5 +- > backends/tpm/tpm_emulator.c | 6 +- > chardev/char.c | 2 - > crypto/afalg.c | 6 +- > hw/hyperv/syndbg.c | 4 +- > io/channel-socket.c | 8 +- > io/channel-watch.c | 10 +- > monitor/fds.c| 77 -- > monitor/hmp-cmds.c | 2 + > monitor/qmp-cmds.c | 7 + > net/dgram.c | 14 +- > net/slirp.c | 16 +- > net/socket.c | 22 +- > tests/qtest/libqtest.c | 26 +- > tests/qtest/microbit-test.c | 2 +- > tests/qtest/netdev-socket.c | 10 +- > tests/qtest/vnc-display-test.c | 12 +- > tests/unit/socket-helpers.c | 2 +- > tests/unit/test-error-report.c | 139 +++ > tests/unit/test-io-channel-command.c | 2 +- > util/aio-posix.c | 6 +- > util/aio-win32.c | 23 +- > util/error.c | 10 +- > util/main-loop.c | 11 - > util/oslib-posix.c | 70 -- > util/oslib-win32.c | 350 --- > util/qemu-sockets.c | 22 +- > hmp-commands.hx | 2 + > tests/docker/docker.py | 6 +- > tests/unit/meson.build | 1 + > 38 files changed, 701 insertions(+), 258 deletions(-) > create mode 100644 tests/unit/test-error-report.c > > -- > 2.39.2 > >
[PULL 13/25] win32: avoid mixing SOCKET and file descriptor space
From: Marc-André Lureau
Until now, a win32 SOCKET handle is often cast to an int file
descriptor, as this is what other OS use for sockets. When necessary,
QEMU eventually queries whether it's a socket with the help of
fd_is_socket(). However, there is no guarantee of conflict between the
fd and SOCKET space. Such conflict would have surprising consequences,
we shouldn't mix them.
Also, it is often forgotten that SOCKET must be closed with
closesocket(), and not close().
Instead, let's make the win32 socket wrapper functions return and take a
file descriptor, and let util/ wrappers do the fd/SOCKET conversion as
necessary. A bit of adaptation is necessary in io/ as well.
Unfortunately, we can't drop closesocket() usage, despite
_open_osfhandle() documentation claiming transfer of ownership, testing
shows bad behaviour if you forget to call closesocket().
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-15-marcandre.lur...@redhat.com>
---
include/sysemu/os-win32.h | 4 +-
io/channel-watch.c| 6 +-
util/aio-win32.c | 9 +-
util/oslib-win32.c| 219 --
4 files changed, 197 insertions(+), 41 deletions(-)
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 504a8966c3..cb1dcce618 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -166,10 +166,10 @@ static inline void qemu_funlockfile(FILE *f)
}
/* Helper for WSAEventSelect, to report errors */
-bool qemu_socket_select(SOCKET s, WSAEVENT hEventObject,
+bool qemu_socket_select(int sockfd, WSAEVENT hEventObject,
long lNetworkEvents, Error **errp);
-bool qemu_socket_unselect(SOCKET s, Error **errp);
+bool qemu_socket_unselect(int sockfd, Error **errp);
/* We wrap all the sockets functions so that we can
* set errno based on WSAGetLastError()
diff --git a/io/channel-watch.c b/io/channel-watch.c
index 6ac41009fa..64b486e378 100644
--- a/io/channel-watch.c
+++ b/io/channel-watch.c
@@ -275,13 +275,13 @@ GSource *qio_channel_create_fd_watch(QIOChannel *ioc,
#ifdef CONFIG_WIN32
GSource *qio_channel_create_socket_watch(QIOChannel *ioc,
- int socket,
+ int sockfd,
GIOCondition condition)
{
GSource *source;
QIOChannelSocketSource *ssource;
-qemu_socket_select(socket, ioc->event,
+qemu_socket_select(sockfd, ioc->event,
FD_READ | FD_ACCEPT | FD_CLOSE |
FD_CONNECT | FD_WRITE | FD_OOB, NULL);
@@ -293,7 +293,7 @@ GSource *qio_channel_create_socket_watch(QIOChannel *ioc,
object_ref(OBJECT(ioc));
ssource->condition = condition;
-ssource->socket = socket;
+ssource->socket = _get_osfhandle(sockfd);
ssource->revents = 0;
ssource->fd.fd = (gintptr)ioc->event;
diff --git a/util/aio-win32.c b/util/aio-win32.c
index 08e8f5615d..6bded009a4 100644
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -73,15 +73,18 @@ void aio_set_fd_handler(AioContext *ctx,
{
AioHandler *old_node;
AioHandler *node = NULL;
+SOCKET s;
if (!fd_is_socket(fd)) {
error_report("fd=%d is not a socket, AIO implementation is missing",
fd);
return;
}
+s = _get_osfhandle(fd);
+
qemu_lockcnt_lock(&ctx->list_lock);
QLIST_FOREACH(old_node, &ctx->aio_handlers, node) {
-if (old_node->pfd.fd == fd && !old_node->deleted) {
+if (old_node->pfd.fd == s && !old_node->deleted) {
break;
}
}
@@ -92,7 +95,7 @@ void aio_set_fd_handler(AioContext *ctx,
/* Alloc and insert if it's not already there */
node = g_new0(AioHandler, 1);
-node->pfd.fd = fd;
+node->pfd.fd = s;
node->pfd.events = 0;
if (node->io_read) {
@@ -120,7 +123,7 @@ void aio_set_fd_handler(AioContext *ctx,
QLIST_INSERT_HEAD_RCU(&ctx->aio_handlers, node, node);
event = event_notifier_get_handle(&ctx->notifier);
-qemu_socket_select(node->pfd.fd, event, bitmask, NULL);
+qemu_socket_select(fd, event, bitmask, NULL);
}
if (old_node) {
aio_remove_fd_handler(ctx, old_node);
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index dbd32acc98..7836fb0be3 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -283,13 +283,20 @@ char *qemu_get_pid_name(pid_t pid)
}
-bool qemu_socket_select(SOCKET s, WSAEVENT hEventObject,
+bool qemu_socket_select(int sockfd, WSAEVENT hEventObject,
long lNetworkEvents, Error **errp)
{
+SOCKET s = _get_osfhandle(sockfd);
+
if (errp == NULL) {
errp = &error_warn;
}
+if (s == INVALID_SOCKET) {
+error_setg(errp, "invalid socket fd=%d", sockfd);
+return false;
+}
+
if (WSAEventSelect(s, hEventObject, lNetworkEvents) != 0) {
e
[PULL 09/25] aio/win32: aio_set_fd_handler() only supports SOCKET
From: Marc-André Lureau
Let's check if the argument is actually a SOCKET, else report an error
and return.
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-10-marcandre.lur...@redhat.com>
---
util/aio-win32.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/util/aio-win32.c b/util/aio-win32.c
index 74d63fa21e..08e8f5615d 100644
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -22,6 +22,7 @@
#include "qemu/sockets.h"
#include "qapi/error.h"
#include "qemu/rcu_queue.h"
+#include "qemu/error-report.h"
struct AioHandler {
EventNotifier *e;
@@ -70,10 +71,14 @@ void aio_set_fd_handler(AioContext *ctx,
IOHandler *io_poll_ready,
void *opaque)
{
-/* fd is a SOCKET in our case */
AioHandler *old_node;
AioHandler *node = NULL;
+if (!fd_is_socket(fd)) {
+error_report("fd=%d is not a socket, AIO implementation is missing",
fd);
+return;
+}
+
qemu_lockcnt_lock(&ctx->list_lock);
QLIST_FOREACH(old_node, &ctx->aio_handlers, node) {
if (old_node->pfd.fd == fd && !old_node->deleted) {
--
2.39.2
[PULL 05/25] error: add global &error_warn destination
From: Marc-André Lureau
This can help debugging issues or develop, when error handling is
introduced.
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-6-marcandre.lur...@redhat.com>
---
include/qapi/error.h | 6 ++
tests/unit/test-error-report.c | 18 ++
util/error.c | 10 +++---
3 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/include/qapi/error.h b/include/qapi/error.h
index d798faeec3..f21a231bb1 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -519,6 +519,12 @@ static inline void
error_propagator_cleanup(ErrorPropagator *prop)
G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(ErrorPropagator, error_propagator_cleanup);
+/*
+ * Special error destination to warn on error.
+ * See error_setg() and error_propagate() for details.
+ */
+extern Error *error_warn;
+
/*
* Special error destination to abort on error.
* See error_setg() and error_propagate() for details.
diff --git a/tests/unit/test-error-report.c b/tests/unit/test-error-report.c
index b09650687b..54319c86c9 100644
--- a/tests/unit/test-error-report.c
+++ b/tests/unit/test-error-report.c
@@ -12,6 +12,7 @@
#include
#include "qemu/error-report.h"
+#include "qapi/error.h"
static void
test_error_report_simple(void)
@@ -103,6 +104,22 @@ test_error_report_timestamp(void)
");
}
+static void
+test_error_warn(void)
+{
+if (g_test_subprocess()) {
+error_setg(&error_warn, "Testing &error_warn");
+return;
+}
+
+g_test_trap_subprocess(NULL, 0, 0);
+g_test_trap_assert_passed();
+g_test_trap_assert_stderr("\
+test-error-report: warning: Testing &error_warn*\
+");
+}
+
+
int
main(int argc, char *argv[])
{
@@ -116,6 +133,7 @@ main(int argc, char *argv[])
g_test_add_func("/error-report/glog", test_error_report_glog);
g_test_add_func("/error-report/once", test_error_report_once);
g_test_add_func("/error-report/timestamp", test_error_report_timestamp);
+g_test_add_func("/error-report/warn", test_error_warn);
return g_test_run();
}
diff --git a/util/error.c b/util/error.c
index 1e7af665b8..5537245da6 100644
--- a/util/error.c
+++ b/util/error.c
@@ -27,8 +27,9 @@ struct Error
Error *error_abort;
Error *error_fatal;
+Error *error_warn;
-static void error_handle_fatal(Error **errp, Error *err)
+static void error_handle(Error **errp, Error *err)
{
if (errp == &error_abort) {
fprintf(stderr, "Unexpected error in %s() at %s:%d:\n",
@@ -43,6 +44,9 @@ static void error_handle_fatal(Error **errp, Error *err)
error_report_err(err);
exit(1);
}
+if (errp == &error_warn) {
+warn_report_err(err);
+}
}
G_GNUC_PRINTF(6, 0)
@@ -71,7 +75,7 @@ static void error_setv(Error **errp,
err->line = line;
err->func = func;
-error_handle_fatal(errp, err);
+error_handle(errp, err);
*errp = err;
errno = saved_errno;
@@ -284,7 +288,7 @@ void error_propagate(Error **dst_errp, Error *local_err)
if (!local_err) {
return;
}
-error_handle_fatal(dst_errp, local_err);
+error_handle(dst_errp, local_err);
if (dst_errp && !*dst_errp) {
*dst_errp = local_err;
} else {
--
2.39.2
[PULL 06/25] win32/socket: introduce qemu_socket_select() helper
From: Marc-André Lureau
This is a wrapper for WSAEventSelect, with Error handling. By default,
it will produce a warning, so callers don't have to be modified
now, and yet we can spot potential mis-use.
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-7-marcandre.lur...@redhat.com>
---
include/sysemu/os-win32.h | 5 +
io/channel-socket.c | 4 ++--
io/channel-watch.c| 6 +++---
util/aio-win32.c | 2 +-
util/main-loop.c | 6 +++---
util/oslib-win32.c| 17 -
6 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 97d0243aee..9f842ae643 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -29,6 +29,7 @@
#include
#include
#include
+#include "qemu/typedefs.h"
#ifdef HAVE_AFUNIX_H
#include
@@ -164,6 +165,10 @@ static inline void qemu_funlockfile(FILE *f)
#endif
}
+/* Helper for WSAEventSelect, to report errors */
+bool qemu_socket_select(SOCKET s, WSAEVENT hEventObject,
+long lNetworkEvents, Error **errp);
+
/* We wrap all the sockets functions so that we can
* set errno based on WSAGetLastError()
*/
diff --git a/io/channel-socket.c b/io/channel-socket.c
index 2040297d2b..0bc29c4808 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -442,7 +442,7 @@ static void qio_channel_socket_finalize(Object *obj)
}
}
#ifdef WIN32
-WSAEventSelect(ioc->fd, NULL, 0);
+qemu_socket_select(ioc->fd, NULL, 0, NULL);
#endif
closesocket(ioc->fd);
ioc->fd = -1;
@@ -846,7 +846,7 @@ qio_channel_socket_close(QIOChannel *ioc,
if (sioc->fd != -1) {
#ifdef WIN32
-WSAEventSelect(sioc->fd, NULL, 0);
+qemu_socket_select(sioc->fd, NULL, 0, NULL);
#endif
if (qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_LISTEN)) {
socket_listen_cleanup(sioc->fd, errp);
diff --git a/io/channel-watch.c b/io/channel-watch.c
index ad7c568a84..6ac41009fa 100644
--- a/io/channel-watch.c
+++ b/io/channel-watch.c
@@ -281,9 +281,9 @@ GSource *qio_channel_create_socket_watch(QIOChannel *ioc,
GSource *source;
QIOChannelSocketSource *ssource;
-WSAEventSelect(socket, ioc->event,
- FD_READ | FD_ACCEPT | FD_CLOSE |
- FD_CONNECT | FD_WRITE | FD_OOB);
+qemu_socket_select(socket, ioc->event,
+ FD_READ | FD_ACCEPT | FD_CLOSE |
+ FD_CONNECT | FD_WRITE | FD_OOB, NULL);
source = g_source_new(&qio_channel_socket_source_funcs,
sizeof(QIOChannelSocketSource));
diff --git a/util/aio-win32.c b/util/aio-win32.c
index 80cfe012ad..be5136e486 100644
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -115,7 +115,7 @@ void aio_set_fd_handler(AioContext *ctx,
QLIST_INSERT_HEAD_RCU(&ctx->aio_handlers, node, node);
event = event_notifier_get_handle(&ctx->notifier);
-WSAEventSelect(node->pfd.fd, event, bitmask);
+qemu_socket_select(node->pfd.fd, event, bitmask, NULL);
}
if (old_node) {
aio_remove_fd_handler(ctx, old_node);
diff --git a/util/main-loop.c b/util/main-loop.c
index 3c0f525192..16e837fb12 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -416,9 +416,9 @@ void qemu_del_wait_object(HANDLE handle, WaitObjectFunc
*func, void *opaque)
void qemu_fd_register(int fd)
{
-WSAEventSelect(fd, event_notifier_get_handle(&qemu_aio_context->notifier),
- FD_READ | FD_ACCEPT | FD_CLOSE |
- FD_CONNECT | FD_WRITE | FD_OOB);
+qemu_socket_select(fd,
event_notifier_get_handle(&qemu_aio_context->notifier),
+ FD_READ | FD_ACCEPT | FD_CLOSE |
+ FD_CONNECT | FD_WRITE | FD_OOB, NULL);
}
static int pollfds_fill(GArray *pollfds, fd_set *rfds, fd_set *wfds,
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 528c9ee156..df752fc762 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -180,7 +180,7 @@ static int socket_error(void)
void qemu_socket_set_block(int fd)
{
unsigned long opt = 0;
-WSAEventSelect(fd, NULL, 0);
+qemu_socket_select(fd, NULL, 0, NULL);
ioctlsocket(fd, FIONBIO, &opt);
}
@@ -283,6 +283,21 @@ char *qemu_get_pid_name(pid_t pid)
}
+bool qemu_socket_select(SOCKET s, WSAEVENT hEventObject,
+long lNetworkEvents, Error **errp)
+{
+if (errp == NULL) {
+errp = &error_warn;
+}
+
+if (WSAEventSelect(s, hEventObject, lNetworkEvents) != 0) {
+error_setg_win32(errp, WSAGetLastError(), "failed to
WSAEventSelect()");
+return false;
+}
+
+return true;
+}
+
#undef connect
int qemu_connect_wrap(int sockfd, const struct sockaddr *addr,
socklen_t addrlen)
--
2.39.2
[PULL 04/25] tests: add test-error-report
From: Marc-André Lureau
Signed-off-by: Marc-André Lureau
Reviewed-by: Stefan Berger
Message-Id: <20230221124802.4103554-5-marcandre.lur...@redhat.com>
---
tests/unit/test-error-report.c | 121 +
tests/unit/meson.build | 1 +
2 files changed, 122 insertions(+)
create mode 100644 tests/unit/test-error-report.c
diff --git a/tests/unit/test-error-report.c b/tests/unit/test-error-report.c
new file mode 100644
index 00..b09650687b
--- /dev/null
+++ b/tests/unit/test-error-report.c
@@ -0,0 +1,121 @@
+/*
+ * Error reporting test
+ *
+ * Copyright (C) 2022 Red Hat Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "glib-compat.h"
+#include
+
+#include "qemu/error-report.h"
+
+static void
+test_error_report_simple(void)
+{
+if (g_test_subprocess()) {
+error_report("%s", "test error");
+warn_report("%s", "test warn");
+info_report("%s", "test info");
+return;
+}
+
+g_test_trap_subprocess(NULL, 0, 0);
+g_test_trap_assert_passed();
+g_test_trap_assert_stderr("\
+test-error-report: test error*\
+test-error-report: warning: test warn*\
+test-error-report: info: test info*\
+");
+}
+
+static void
+test_error_report_loc(void)
+{
+if (g_test_subprocess()) {
+loc_set_file("some-file.c", 7717);
+error_report("%s", "test error1");
+loc_set_none();
+error_report("%s", "test error2");
+return;
+}
+
+g_test_trap_subprocess(NULL, 0, 0);
+g_test_trap_assert_passed();
+g_test_trap_assert_stderr("\
+test-error-report:some-file.c:7717: test error1*\
+test-error-report: test error2*\
+");
+}
+
+static void
+test_error_report_glog(void)
+{
+if (g_test_subprocess()) {
+g_message("gmessage");
+return;
+}
+
+g_test_trap_subprocess(NULL, 0, 0);
+g_test_trap_assert_passed();
+g_test_trap_assert_stderr("test-error-report: info: gmessage*");
+}
+
+static void
+test_error_report_once(void)
+{
+int i;
+
+if (g_test_subprocess()) {
+for (i = 0; i < 3; i++) {
+warn_report_once("warn");
+error_report_once("err");
+}
+return;
+}
+
+g_test_trap_subprocess(NULL, 0, 0);
+g_test_trap_assert_passed();
+g_test_trap_assert_stderr("\
+test-error-report: warning: warn*\
+test-error-report: err*\
+");
+}
+
+static void
+test_error_report_timestamp(void)
+{
+if (g_test_subprocess()) {
+message_with_timestamp = true;
+warn_report("warn");
+error_report("err");
+return;
+}
+
+g_test_trap_subprocess(NULL, 0, 0);
+g_test_trap_assert_passed();
+g_test_trap_assert_stderr("\
+*-*-*:*:* test-error-report: warning: warn*\
+*-*-*:*:* test-error-report: err*\
+");
+}
+
+int
+main(int argc, char *argv[])
+{
+setlocale(LC_ALL, "");
+
+g_test_init(&argc, &argv, NULL);
+error_init("test-error-report");
+
+g_test_add_func("/error-report/simple", test_error_report_simple);
+g_test_add_func("/error-report/loc", test_error_report_loc);
+g_test_add_func("/error-report/glog", test_error_report_glog);
+g_test_add_func("/error-report/once", test_error_report_once);
+g_test_add_func("/error-report/timestamp", test_error_report_timestamp);
+
+return g_test_run();
+}
diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index d9c0a7eae6..fa63cfe6ff 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -11,6 +11,7 @@ tests = {
'check-qobject': [],
'check-qjson': [],
'check-qlit': [],
+ 'test-error-report': [],
'test-qobject-output-visitor': [testqapi],
'test-clone-visitor': [testqapi],
'test-qobject-input-visitor': [testqapi],
--
2.39.2
[PULL 00/25] Win socket patches
From: Marc-André Lureau The following changes since commit 29c8a9e31a982874ce4e2c15f2bf82d5f8dc3517: Merge tag 'linux-user-for-8.0-pull-request' of https://gitlab.com/laurent_vivier/qemu into staging (2023-03-12 10:57:00 +) are available in the Git repository at: https://gitlab.com/marcandre.lureau/qemu.git tags/win-socket-pull-request for you to fetch changes up to 0006c18362cbe1e93587ac1e8ee965c08e6970f2: QMP/HMP: only actually implement getfd on CONFIG_POSIX (2023-03-13 15:41:32 +0400) QMP command to import win32 sockets Marc-André Lureau (25): util: drop qemu_fork() tests: use closesocket() io: use closesocket() tests: add test-error-report error: add global &error_warn destination win32/socket: introduce qemu_socket_select() helper win32/socket: introduce qemu_socket_unselect() helper aio: make aio_set_fd_poll() static to aio-posix.c aio/win32: aio_set_fd_handler() only supports SOCKET main-loop: remove qemu_fd_register(), win32/slirp/socket specific slirp: unregister the win32 SOCKET slirp: open-code qemu_socket_(un)select() win32: avoid mixing SOCKET and file descriptor space os-posix: remove useless ioctlsocket() define win32: replace closesocket() with close() wrapper tests: fix path separator, use g_build_filename() char: do not double-close fd when failing to add client tests/docker: fix a win32 error due to portability osdep: implement qemu_socketpair() for win32 qmp: 'add_client' actually expects sockets monitor: release the lock before calling close() qmp: add 'get-win32-socket' libqtest: make qtest_qmp_add_client work on win32 qtest: enable vnc-display test on win32 QMP/HMP: only actually implement getfd on CONFIG_POSIX qapi/misc.json | 36 ++- include/block/aio.h | 8 - include/qapi/error.h | 6 + include/qemu/main-loop.h | 2 - include/qemu/osdep.h | 14 -- include/qemu/sockets.h | 2 - include/sysemu/os-posix.h| 3 - include/sysemu/os-win32.h| 15 +- tests/qtest/libqtest.h | 5 +- backends/tpm/tpm_emulator.c | 6 +- chardev/char.c | 2 - crypto/afalg.c | 6 +- hw/hyperv/syndbg.c | 4 +- io/channel-socket.c | 8 +- io/channel-watch.c | 10 +- monitor/fds.c| 77 -- monitor/hmp-cmds.c | 2 + monitor/qmp-cmds.c | 7 + net/dgram.c | 14 +- net/slirp.c | 16 +- net/socket.c | 22 +- tests/qtest/libqtest.c | 26 +- tests/qtest/microbit-test.c | 2 +- tests/qtest/netdev-socket.c | 10 +- tests/qtest/vnc-display-test.c | 12 +- tests/unit/socket-helpers.c | 2 +- tests/unit/test-error-report.c | 139 +++ tests/unit/test-io-channel-command.c | 2 +- util/aio-posix.c | 6 +- util/aio-win32.c | 23 +- util/error.c | 10 +- util/main-loop.c | 11 - util/oslib-posix.c | 70 -- util/oslib-win32.c | 350 --- util/qemu-sockets.c | 22 +- hmp-commands.hx | 2 + tests/docker/docker.py | 6 +- tests/unit/meson.build | 1 + 38 files changed, 701 insertions(+), 258 deletions(-) create mode 100644 tests/unit/test-error-report.c -- 2.39.2
Re: [PATCH v10 00/12] parallels: Refactor the code of images checks and fix a bug
On 3/7/23 13:20, Hanna Czenczek wrote: On 03.02.23 10:18, Alexander Ivanov wrote: Fix image inflation when offset in BAT is out of image. Replace whole BAT syncing by flushing only dirty blocks. Move all the checks outside the main check function in separate functions Use WITH_QEMU_LOCK_GUARD for simplier code. Fix incorrect condition in out-of-image check. Apart from my comments (and I think regardless of what happens for patch 1), looks good to me! Some patches will need to be rebased on the GRAPH_RDLOCK changes, but that looks straightforward. Hanna I think that I should pass the process of learning how to send pull request and make it as soon as possible in order to learn. We are really good enough. Den
Re: [PATCH] include/blcok: fixup typos
On Mon, 13 Mar 2023 at 00:26, Wilfred Mallawa wrote: > > From: Wilfred Mallawa > > Fixup a few minor typos Typo in patch subject line: should be 'block' :-) > Signed-off-by: Wilfred Mallawa > --- Otherwise Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v7 4/6] hw: replace most qemu_bh_new calls with qemu_bh_new_guarded
On 13/03/2023 09.24, Alexander Bulekov wrote: This protects devices from bh->mmio reentrancy issues. Thanks: Thomas Huth for diagnosing OS X test failure. Reviewed-by: Darren Kenny Reviewed-by: Stefan Hajnoczi Reviewed-by: Michael S. Tsirkin Reviewed-by: Paul Durrant Signed-off-by: Alexander Bulekov --- hw/9pfs/xen-9p-backend.c| 5 - hw/block/dataplane/virtio-blk.c | 3 ++- hw/block/dataplane/xen-block.c | 5 +++-- hw/char/virtio-serial-bus.c | 3 ++- hw/display/qxl.c| 9 ++--- hw/display/virtio-gpu.c | 6 -- hw/ide/ahci.c | 3 ++- hw/ide/ahci_internal.h | 1 + hw/ide/core.c | 4 +++- hw/misc/imx_rngc.c | 6 -- hw/misc/macio/mac_dbdma.c | 2 +- hw/net/virtio-net.c | 3 ++- hw/nvme/ctrl.c | 6 -- hw/scsi/mptsas.c| 3 ++- hw/scsi/scsi-bus.c | 3 ++- hw/scsi/vmw_pvscsi.c| 3 ++- hw/usb/dev-uas.c| 3 ++- hw/usb/hcd-dwc2.c | 3 ++- hw/usb/hcd-ehci.c | 3 ++- hw/usb/hcd-uhci.c | 2 +- hw/usb/host-libusb.c| 6 -- hw/usb/redirect.c | 6 -- hw/usb/xen-usb.c| 3 ++- hw/virtio/virtio-balloon.c | 5 +++-- hw/virtio/virtio-crypto.c | 3 ++- 25 files changed, 66 insertions(+), 33 deletions(-) Reviewed-by: Thomas Huth
[PATCH v7 2/6] async: Add an optional reentrancy guard to the BH API
Devices can pass their MemoryReentrancyGuard (from their DeviceState),
when creating new BHes. Then, the async API will toggle the guard
before/after calling the BH call-back. This prevents bh->mmio reentrancy
issues.
Reviewed-by: Darren Kenny
Signed-off-by: Alexander Bulekov
---
docs/devel/multiple-iothreads.txt | 7 +++
include/block/aio.h | 18 --
include/qemu/main-loop.h | 7 +--
tests/unit/ptimer-test-stubs.c| 3 ++-
util/async.c | 18 +-
util/main-loop.c | 5 +++--
util/trace-events | 1 +
7 files changed, 51 insertions(+), 8 deletions(-)
diff --git a/docs/devel/multiple-iothreads.txt
b/docs/devel/multiple-iothreads.txt
index 343120f2ef..a3e949f6b3 100644
--- a/docs/devel/multiple-iothreads.txt
+++ b/docs/devel/multiple-iothreads.txt
@@ -61,6 +61,7 @@ There are several old APIs that use the main loop AioContext:
* LEGACY qemu_aio_set_event_notifier() - monitor an event notifier
* LEGACY timer_new_ms() - create a timer
* LEGACY qemu_bh_new() - create a BH
+ * LEGACY qemu_bh_new_guarded() - create a BH with a device re-entrancy guard
* LEGACY qemu_aio_wait() - run an event loop iteration
Since they implicitly work on the main loop they cannot be used in code that
@@ -72,8 +73,14 @@ Instead, use the AioContext functions directly (see
include/block/aio.h):
* aio_set_event_notifier() - monitor an event notifier
* aio_timer_new() - create a timer
* aio_bh_new() - create a BH
+ * aio_bh_new_guarded() - create a BH with a device re-entrancy guard
* aio_poll() - run an event loop iteration
+The qemu_bh_new_guarded/aio_bh_new_guarded APIs accept a "MemReentrancyGuard"
+argument, which is used to check for and prevent re-entrancy problems. For
+BHs associated with devices, the reentrancy-guard is contained in the
+corresponding DeviceState and named "mem_reentrancy_guard".
+
The AioContext can be obtained from the IOThread using
iothread_get_aio_context() or for the main loop using qemu_get_aio_context().
Code that takes an AioContext argument works both in IOThreads or the main
diff --git a/include/block/aio.h b/include/block/aio.h
index 8fba6a3584..3e3bdb9352 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -23,6 +23,8 @@
#include "qemu/thread.h"
#include "qemu/timer.h"
#include "block/graph-lock.h"
+#include "hw/qdev-core.h"
+
typedef struct BlockAIOCB BlockAIOCB;
typedef void BlockCompletionFunc(void *opaque, int ret);
@@ -331,9 +333,11 @@ void aio_bh_schedule_oneshot_full(AioContext *ctx,
QEMUBHFunc *cb, void *opaque,
* is opaque and must be allocated prior to its use.
*
* @name: A human-readable identifier for debugging purposes.
+ * @reentrancy_guard: A guard set when entering a cb to prevent
+ * device-reentrancy issues
*/
QEMUBH *aio_bh_new_full(AioContext *ctx, QEMUBHFunc *cb, void *opaque,
-const char *name);
+const char *name, MemReentrancyGuard
*reentrancy_guard);
/**
* aio_bh_new: Allocate a new bottom half structure
@@ -342,7 +346,17 @@ QEMUBH *aio_bh_new_full(AioContext *ctx, QEMUBHFunc *cb,
void *opaque,
* string.
*/
#define aio_bh_new(ctx, cb, opaque) \
-aio_bh_new_full((ctx), (cb), (opaque), (stringify(cb)))
+aio_bh_new_full((ctx), (cb), (opaque), (stringify(cb)), NULL)
+
+/**
+ * aio_bh_new_guarded: Allocate a new bottom half structure with a
+ * reentrancy_guard
+ *
+ * A convenience wrapper for aio_bh_new_full() that uses the cb as the name
+ * string.
+ */
+#define aio_bh_new_guarded(ctx, cb, opaque, guard) \
+aio_bh_new_full((ctx), (cb), (opaque), (stringify(cb)), guard)
/**
* aio_notify: Force processing of pending events.
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index c25f390696..84d1ce57f0 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -389,9 +389,12 @@ void qemu_cond_timedwait_iothread(QemuCond *cond, int ms);
void qemu_fd_register(int fd);
+#define qemu_bh_new_guarded(cb, opaque, guard) \
+qemu_bh_new_full((cb), (opaque), (stringify(cb)), guard)
#define qemu_bh_new(cb, opaque) \
-qemu_bh_new_full((cb), (opaque), (stringify(cb)))
-QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb, void *opaque, const char *name);
+qemu_bh_new_full((cb), (opaque), (stringify(cb)), NULL)
+QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb, void *opaque, const char *name,
+ MemReentrancyGuard *reentrancy_guard);
void qemu_bh_schedule_idle(QEMUBH *bh);
enum {
diff --git a/tests/unit/ptimer-test-stubs.c b/tests/unit/ptimer-test-stubs.c
index f2bfcede93..8c9407c560 100644
--- a/tests/unit/ptimer-test-stubs.c
+++ b/tests/unit/ptimer-test-stubs.c
@@ -107,7 +107,8 @@ int64_t qemu_clock_deadline_ns_all(QEMUClockType type, int
attr_mask)
return deadline;
}
-QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb, void *opaque, const char *name)
+QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb,
[PATCH v7 4/6] hw: replace most qemu_bh_new calls with qemu_bh_new_guarded
This protects devices from bh->mmio reentrancy issues.
Thanks: Thomas Huth for diagnosing OS X test failure.
Reviewed-by: Darren Kenny
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Michael S. Tsirkin
Reviewed-by: Paul Durrant
Signed-off-by: Alexander Bulekov
---
hw/9pfs/xen-9p-backend.c| 5 -
hw/block/dataplane/virtio-blk.c | 3 ++-
hw/block/dataplane/xen-block.c | 5 +++--
hw/char/virtio-serial-bus.c | 3 ++-
hw/display/qxl.c| 9 ++---
hw/display/virtio-gpu.c | 6 --
hw/ide/ahci.c | 3 ++-
hw/ide/ahci_internal.h | 1 +
hw/ide/core.c | 4 +++-
hw/misc/imx_rngc.c | 6 --
hw/misc/macio/mac_dbdma.c | 2 +-
hw/net/virtio-net.c | 3 ++-
hw/nvme/ctrl.c | 6 --
hw/scsi/mptsas.c| 3 ++-
hw/scsi/scsi-bus.c | 3 ++-
hw/scsi/vmw_pvscsi.c| 3 ++-
hw/usb/dev-uas.c| 3 ++-
hw/usb/hcd-dwc2.c | 3 ++-
hw/usb/hcd-ehci.c | 3 ++-
hw/usb/hcd-uhci.c | 2 +-
hw/usb/host-libusb.c| 6 --
hw/usb/redirect.c | 6 --
hw/usb/xen-usb.c| 3 ++-
hw/virtio/virtio-balloon.c | 5 +++--
hw/virtio/virtio-crypto.c | 3 ++-
25 files changed, 66 insertions(+), 33 deletions(-)
diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
index 74f3a05f88..0e266c552b 100644
--- a/hw/9pfs/xen-9p-backend.c
+++ b/hw/9pfs/xen-9p-backend.c
@@ -61,6 +61,7 @@ typedef struct Xen9pfsDev {
int num_rings;
Xen9pfsRing *rings;
+MemReentrancyGuard mem_reentrancy_guard;
} Xen9pfsDev;
static void xen_9pfs_disconnect(struct XenLegacyDevice *xendev);
@@ -443,7 +444,9 @@ static int xen_9pfs_connect(struct XenLegacyDevice *xendev)
xen_9pdev->rings[i].ring.out = xen_9pdev->rings[i].data +
XEN_FLEX_RING_SIZE(ring_order);
-xen_9pdev->rings[i].bh = qemu_bh_new(xen_9pfs_bh,
&xen_9pdev->rings[i]);
+xen_9pdev->rings[i].bh = qemu_bh_new_guarded(xen_9pfs_bh,
+ &xen_9pdev->rings[i],
+
&xen_9pdev->mem_reentrancy_guard);
xen_9pdev->rings[i].out_cons = 0;
xen_9pdev->rings[i].out_size = 0;
xen_9pdev->rings[i].inprogress = false;
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index b28d81737e..a6202997ee 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -127,7 +127,8 @@ bool virtio_blk_data_plane_create(VirtIODevice *vdev,
VirtIOBlkConf *conf,
} else {
s->ctx = qemu_get_aio_context();
}
-s->bh = aio_bh_new(s->ctx, notify_guest_bh, s);
+s->bh = aio_bh_new_guarded(s->ctx, notify_guest_bh, s,
+ &DEVICE(vdev)->mem_reentrancy_guard);
s->batch_notify_vqs = bitmap_new(conf->num_queues);
*dataplane = s;
diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c
index 734da42ea7..d8bc39d359 100644
--- a/hw/block/dataplane/xen-block.c
+++ b/hw/block/dataplane/xen-block.c
@@ -633,8 +633,9 @@ XenBlockDataPlane *xen_block_dataplane_create(XenDevice
*xendev,
} else {
dataplane->ctx = qemu_get_aio_context();
}
-dataplane->bh = aio_bh_new(dataplane->ctx, xen_block_dataplane_bh,
- dataplane);
+dataplane->bh = aio_bh_new_guarded(dataplane->ctx, xen_block_dataplane_bh,
+ dataplane,
+ &DEVICE(xendev)->mem_reentrancy_guard);
return dataplane;
}
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 7d4601cb5d..dd619f0731 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -985,7 +985,8 @@ static void virtser_port_device_realize(DeviceState *dev,
Error **errp)
return;
}
-port->bh = qemu_bh_new(flush_queued_data_bh, port);
+port->bh = qemu_bh_new_guarded(flush_queued_data_bh, port,
+ &dev->mem_reentrancy_guard);
port->elem = NULL;
}
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index ec712d3ca2..c0460c4ef1 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -2201,11 +2201,14 @@ static void qxl_realize_common(PCIQXLDevice *qxl, Error
**errp)
qemu_add_vm_change_state_handler(qxl_vm_change_state_handler, qxl);
-qxl->update_irq = qemu_bh_new(qxl_update_irq_bh, qxl);
+qxl->update_irq = qemu_bh_new_guarded(qxl_update_irq_bh, qxl,
+ &DEVICE(qxl)->mem_reentrancy_guard);
qxl_reset_state(qxl);
-qxl->update_area_bh = qemu_bh_new(qxl_render_update_area_bh, qxl);
-qxl->ssd.cursor_bh = qemu_bh_new(qemu_spice_cursor_refresh_bh, &qxl->ssd);
+qxl->update_area_bh = qemu_bh_new_guarded(qxl_render_update_ar
Re: [PATCH v2] include/block: fixup typos
On 13/3/23 01:37, Wilfred Mallawa wrote: From: Wilfred Mallawa Fixup a few minor typos Signed-off-by: Wilfred Mallawa --- v2: - Fixup typo in commit msg. include/block/aio-wait.h | 2 +- include/block/block_int-common.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h index da13357bb8..6e43e3b7bb 100644 --- a/include/block/aio-wait.h +++ b/include/block/aio-wait.h @@ -63,7 +63,7 @@ extern AioWait global_aio_wait; * @ctx: the aio context, or NULL if multiple aio contexts (for which the * caller does not hold a lock) are involved in the polling condition. * @cond: wait while this conditional expression is true - * @unlock: whether to unlock and then lock again @ctx. This apples :) Reviewed-by: Philippe Mathieu-Daudé + * @unlock: whether to unlock and then lock again @ctx. This applies * only when waiting for another AioContext from the main loop. * Otherwise it's ignored.
