[PATCH v4] file-posix: detect the lock using the real file
This patch addresses this issue:
When accessing a volume on an NFS filesystem without supporting the file lock,
tools, like qemu-img, will complain "Failed to lock byte 100".
In the original code, the qemu_has_ofd_lock will test the lock on the
"/dev/null" pseudo-file. Actually, the file.locking is per-drive property,
which depends on the underlay filesystem.
In this patch, add a new 'qemu_has_file_lock' to detect whether the
file supports the file lock. And disable the lock when the underlay file
system doesn't support locks.
Signed-off-by: Li Feng
---
v4: use the fd as the qemu_has_file_lock argument.
v3: don't call the qemu_has_ofd_lock, use a new function instead.
v2: remove the refactoring.
---
block/file-posix.c | 66 +---
include/qemu/osdep.h | 1 +
util/osdep.c | 19 +
3 files changed, 58 insertions(+), 28 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 806764f7e3..9708212f01 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -584,6 +584,21 @@ static int raw_open_common(BlockDriverState *bs, QDict
*options,
s->use_linux_io_uring = (aio == BLOCKDEV_AIO_OPTIONS_IO_URING);
#endif
+s->open_flags = open_flags;
+raw_parse_flags(bdrv_flags, &s->open_flags, false);
+
+s->fd = -1;
+fd = qemu_open(filename, s->open_flags, errp);
+ret = fd < 0 ? -errno : 0;
+
+if (ret < 0) {
+if (ret == -EROFS) {
+ret = -EACCES;
+}
+goto fail;
+}
+s->fd = fd;
+
locking = qapi_enum_parse(&OnOffAuto_lookup,
qemu_opt_get(opts, "locking"),
ON_OFF_AUTO_AUTO, &local_err);
@@ -607,6 +622,13 @@ static int raw_open_common(BlockDriverState *bs, QDict
*options,
break;
case ON_OFF_AUTO_AUTO:
s->use_lock = qemu_has_ofd_lock();
+if (s->use_lock && !qemu_has_file_lock(s->fd)) {
+/*
+ * When the os supports the OFD lock, but the filesystem doesn't
+ * support, just disable the file lock.
+ */
+s->use_lock = false;
+}
break;
default:
abort();
@@ -625,22 +647,6 @@ static int raw_open_common(BlockDriverState *bs, QDict
*options,
s->drop_cache = qemu_opt_get_bool(opts, "drop-cache", true);
s->check_cache_dropped = qemu_opt_get_bool(opts, "x-check-cache-dropped",
false);
-
-s->open_flags = open_flags;
-raw_parse_flags(bdrv_flags, &s->open_flags, false);
-
-s->fd = -1;
-fd = qemu_open(filename, s->open_flags, errp);
-ret = fd < 0 ? -errno : 0;
-
-if (ret < 0) {
-if (ret == -EROFS) {
-ret = -EACCES;
-}
-goto fail;
-}
-s->fd = fd;
-
/* Check s->open_flags rather than bdrv_flags due to auto-read-only */
if (s->open_flags & O_RDWR) {
ret = check_hdev_writable(s->fd);
@@ -2388,6 +2394,7 @@ raw_co_create(BlockdevCreateOptions *options, Error
**errp)
int fd;
uint64_t perm, shared;
int result = 0;
+bool use_lock;
/* Validate options and set default values */
assert(options->driver == BLOCKDEV_DRIVER_FILE);
@@ -2428,19 +2435,22 @@ raw_co_create(BlockdevCreateOptions *options, Error
**errp)
perm = BLK_PERM_WRITE | BLK_PERM_RESIZE;
shared = BLK_PERM_ALL & ~BLK_PERM_RESIZE;
-/* Step one: Take locks */
-result = raw_apply_lock_bytes(NULL, fd, perm, ~shared, false, errp);
-if (result < 0) {
-goto out_close;
-}
+use_lock = qemu_has_file_lock(fd);
+if (use_lock) {
+/* Step one: Take locks */
+result = raw_apply_lock_bytes(NULL, fd, perm, ~shared, false, errp);
+if (result < 0) {
+goto out_close;
+}
-/* Step two: Check that nobody else has taken conflicting locks */
-result = raw_check_lock_bytes(fd, perm, shared, errp);
-if (result < 0) {
-error_append_hint(errp,
- "Is another process using the image [%s]?\n",
- file_opts->filename);
-goto out_unlock;
+/* Step two: Check that nobody else has taken conflicting locks */
+result = raw_check_lock_bytes(fd, perm, shared, errp);
+if (result < 0) {
+error_append_hint(errp,
+ "Is another process using the image [%s]?\n",
+ file_opts->filename);
+goto out_unlock;
+}
}
/* Clear the file by truncating it to 0 */
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index f9ec8c84e9..c7587be99d 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -513,6 +513,7 @@ int qemu_lock_fd(int fd, int64_t start, int64_t len, bool
exclusive);
int qemu_unlock_fd(int fd, int64_t start, int64_t len);
int qemu_lock_fd_test(int fd, int64_t start, int64_t len, bool exclusive);
bool qemu_has_ofd_loc
Re: [PATCH v3] file-posix: detect the lock using the real file
Hi, Daniel
Thanks for your reply.
I have just ended my trip, sorry for my late response.
I will send out the v4.
Daniel P. Berrangé 于2020年12月11日周五 上午12:55写道：
>
> On Fri, Dec 11, 2020 at 12:38:19AM +0800, Li Feng wrote:
> > This patch addresses this issue:
> > When accessing a volume on an NFS filesystem without supporting the file
> > lock,
> > tools, like qemu-img, will complain "Failed to lock byte 100".
> >
> > Add a new function 'qemu_has_file_lock' to detect if the filesystem
> > supports locks
> > or not.
> > And when the drive is auto mode, use the 'qemu_has_file_lock' to set the
> > toggle.
> >
> > Signed-off-by: Li Feng
> > ---
> > v3: don't call the qemu_has_ofd_lock, use a new function instead.
> > v2: remove the refactoring.
> > ---
> > block/file-posix.c | 30 +-
> > include/qemu/osdep.h | 1 +
> > util/osdep.c | 29 +
> > 3 files changed, 47 insertions(+), 13 deletions(-)
> >
> > diff --git a/block/file-posix.c b/block/file-posix.c
> > index 806764f7e3..48f9a32de2 100644
> > --- a/block/file-posix.c
> > +++ b/block/file-posix.c
> > @@ -606,7 +606,7 @@ static int raw_open_common(BlockDriverState *bs, QDict
> > *options,
> > s->use_lock = false;
> > break;
> > case ON_OFF_AUTO_AUTO:
> > -s->use_lock = qemu_has_ofd_lock();
> > +s->use_lock = qemu_has_file_lock(filename);
>
> This is not good - it causes us to always use locks by default, where
> as previously we only used them if OFD was available. It neds to test
> both here, except opening + closing filename to test for fnctl support
> risks releasing any locks QEMU already holds on filename if OFD is not
> supported.
Yes, check the qemu_has_ofd_lock and qemu_has_file_lock both, and
set the use_lock to false when the os supports the OFD lock, but the
filesystem doesn't support.
>
> > break;
> > default:
> > abort();
> > @@ -2388,6 +2388,7 @@ raw_co_create(BlockdevCreateOptions *options, Error
> > **errp)
> > int fd;
> > uint64_t perm, shared;
> > int result = 0;
> > +bool use_lock;
> >
> > /* Validate options and set default values */
> > assert(options->driver == BLOCKDEV_DRIVER_FILE);
> > @@ -2428,19 +2429,22 @@ raw_co_create(BlockdevCreateOptions *options, Error
> > **errp)
> > perm = BLK_PERM_WRITE | BLK_PERM_RESIZE;
> > shared = BLK_PERM_ALL & ~BLK_PERM_RESIZE;
> >
> > -/* Step one: Take locks */
> > -result = raw_apply_lock_bytes(NULL, fd, perm, ~shared, false, errp);
> > -if (result < 0) {
> > -goto out_close;
> > -}
> > +use_lock = qemu_has_file_lock(file_opts->filename);
>
> This cause QEMU to open and close filename. If another thread
> already had filename open, and OFD is not support, we've just
> lock the locks we held. We need to use 'fd' which is already
> open.
Acked.
>
> > +if (use_lock) {
> > +/* Step one: Take locks */
> > +result = raw_apply_lock_bytes(NULL, fd, perm, ~shared, false,
> > errp);
> > +if (result < 0) {
> > +goto out_close;
> > +}
> >
> > -/* Step two: Check that nobody else has taken conflicting locks */
> > -result = raw_check_lock_bytes(fd, perm, shared, errp);
> > -if (result < 0) {
> > -error_append_hint(errp,
> > - "Is another process using the image [%s]?\n",
> > - file_opts->filename);
> > -goto out_unlock;
> > +/* Step two: Check that nobody else has taken conflicting locks */
> > +result = raw_check_lock_bytes(fd, perm, shared, errp);
> > +if (result < 0) {
> > +error_append_hint(errp,
> > + "Is another process using the image [%s]?\n",
> > + file_opts->filename);
> > +goto out_unlock;
> > +}
> > }
> >
> > /* Clear the file by truncating it to 0 */
>
>
> > +bool qemu_has_file_lock(const char *filename)
>
> IMO thisshould just accept a pre-opened 'int fd'
Acked.
>
> > +{
> > +#ifdef F_OFD_SETLK
> > +int cmd = F_OFD_GETLK;
> > +#else
> > +int cmd = F_GETLK;
> > +#endif
> > +int fd;
> > +int ret;
> > +struct flock fl = {
> > +.l_whence = SEEK_SET,
> > +.l_start = 0,
> > +.l_len= 0,
> > +.l_type = F_WRLCK,
> > +};
> > +
> > +fd = open(filename, O_RDWR);
> > +if (fd < 0) {
> > +fprintf(stderr,
> > +"Failed to open %s for OFD lock probing: %s\n",
> > +filename,
> > +strerror(errno));
> > +return false;
> > +}
> > +ret = fcntl(fd, cmd, &fl);
> > +close(fd);
> > +return ret == 0;
> > +}
> > +
> > bool qemu_has_ofd_lock(void)
> > {
> > qemu_probe_lock_ops();
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o-https:
RE: [PATCH v5 0/4] hw/block/m25p80: Numonyx: Fix dummy cycles and check for SPI mode on cmds
Hi Peter, This series has been reviewed, but it looks like it slipped through the cracks. Is it possible it could be merged through your tree, assuming it looks good? Thanks! Joe -Original Message- From: Qemu-devel On Behalf Of Joe Komlodi Sent: Monday, November 16, 2020 3:11 PM To: qemu-de...@nongnu.org Cc: Francisco Eduardo Iglesias ; alist...@alistair23.me; philippe.mathieu.da...@gmail.com; qemu-block@nongnu.org; mre...@redhat.com Subject: [PATCH v5 0/4] hw/block/m25p80: Numonyx: Fix dummy cycles and check for SPI mode on cmds Changelog: v4 -> v5 - 3/4: Simplify logic when changing state and checking mode. - 3/4: numonyx_get_mode -> numonyx_mode - 4/4: Reword commit message to include QIO mode. v3 -> v4 - 1/4: Patch changed to change names of register fields to be more accurate. - 1/4: Revert polarity change from v3. - 2/4: Added, fixes polarity of VCFG XIP mode when copied from NVCFG. - 3/4: Removed check_cmd_mode function, each command check is done in decode_new_cmd instead. - 3/4: Add guest error print if JEDEC read is executed in QIO or DIO mode. - 3/4: Don't check PP and PP4, they work regardless of mode. PP4_4 is left as is. - 3/4: Simplify get_mode function. - 4/4: Simplify extract_cfg_num_dummies function. - 4/4: Use switch statement instead of table for cycle retrieving. v2 -> v3 - 1/3: Added, Fixes NVCFG polarity for DIO/QIO. - 2/3: Added, Checks if we can execute the current command in standard/DIO/QIO mode. - 3/3: Was 1/1 in v2. Added cycle counts for DIO/QIO mode. v1 -> v2 - 1/2: Change function name to be more accurate - 2/2: Dropped Hi all, The series fixes the behavior of the dummy cycle register for Numonyx flashes so it's closer to how hardware behaves. It also checks if a command can be executed in the current SPI mode (standard, DIO, or QIO) before extracting dummy cycles for the command. On hardware, the dummy cycles for fast read commands are set to a specific value (8 or 10) if the register is all 0s or 1s. If the register value isn't all 0s or 1s, then the flash expects the amount of cycles sent to be equal to the count in the register. Thanks! Joe Joe Komlodi (4): hw/block/m25p80: Make Numonyx config field names more accurate hw/block/m25p80: Fix when VCFG XIP bit is set for Numonyx hw/block/m25p80: Check SPI mode before running some Numonyx commands hw/block/m25p80: Fix Numonyx fast read dummy cycle count hw/block/m25p80.c | 158 -- 1 file changed, 129 insertions(+), 29 deletions(-) -- 2.7.4
[PATCH] iotests: Quote $cmd in _send_qemu_cmd
With bash 5.1, the output of the following script (which creates an
array with a single element, then takes a single-element slice from that
array, and echos the result) changes:
a=("double space")
a=${a[@]:0:1}
echo "$a"
from "double space" to "double space", i.e. all white space is
preserved as-is. This is probably what we actually want here (judging
from the "...to accommodate pathnames with spaces" comment), but before
5.1, we have to quote the ${} slice to get the same behavior.
In any case, without quoting, the reference output of many iotests is
different between bash 5.1 and pre-5.1, which is not very good. We
should fix it by quoting here, and then adjusting all the reference
output of all iotests using _send_qemu_cmd accordingly. (The only
ones we do not need to change are those that do not have multiple
consecutive whitespaces in their _send_qemu_cmd parameters.)
Signed-off-by: Max Reitz
---
Alternatively, we could explicitly collapse all whitespace sequences
into single spaces, but I believe that would defeat the purpose of
"accomodat[ing] pathnames with spaces".
I used this script to verify that the git-diff only changes whitespace
(except for the hunk in common.qemu):
https://gist.github.com/XanClic/41cfcc2ac4619421883e8a97790f5c9e
---
tests/qemu-iotests/085.out | 167 -
tests/qemu-iotests/094.out | 10 +-
tests/qemu-iotests/095.out | 4 +-
tests/qemu-iotests/109.out | 88 -
tests/qemu-iotests/117.out | 13 ++-
tests/qemu-iotests/127.out | 12 ++-
tests/qemu-iotests/140.out | 10 +-
tests/qemu-iotests/141.out | 128 +++--
tests/qemu-iotests/143.out | 4 +-
tests/qemu-iotests/144.out | 28 +-
tests/qemu-iotests/153.out | 18 ++--
tests/qemu-iotests/156.out | 39 ++--
tests/qemu-iotests/161.out | 18 +++-
tests/qemu-iotests/173.out | 25 -
tests/qemu-iotests/182.out | 42 +++--
tests/qemu-iotests/183.out | 19 +++-
tests/qemu-iotests/185.out | 45 +++--
tests/qemu-iotests/191.out | 12 ++-
tests/qemu-iotests/223.out | 92 --
tests/qemu-iotests/229.out | 13 ++-
tests/qemu-iotests/249.out | 16 +++-
tests/qemu-iotests/308.out | 103 +---
tests/qemu-iotests/312.out | 10 +-
tests/qemu-iotests/common.qemu | 4 +-
24 files changed, 727 insertions(+), 193 deletions(-)
diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out
index 7fc44b1c61..32a193f2c2 100644
--- a/tests/qemu-iotests/085.out
+++ b/tests/qemu-iotests/085.out
@@ -12,56 +12,135 @@ Formatting 'TEST_DIR/t.IMGFMT.2', fmt=IMGFMT size=134217728
=== Create a single snapshot on virtio0 ===
-{ 'execute': 'blockdev-snapshot-sync', 'arguments': { 'device': 'virtio0',
'snapshot-file':'TEST_DIR/1-snapshot-v0.IMGFMT', 'format': 'IMGFMT' } }
+{ 'execute': 'blockdev-snapshot-sync',
+ 'arguments': { 'device': 'virtio0',
+
'snapshot-file':'TEST_DIR/1-snapshot-v0.IMGFMT',
+ 'format': 'IMGFMT' } }
Formatting 'TEST_DIR/1-snapshot-v0.qcow2', fmt=qcow2 cluster_size=65536
extended_l2=off compression_type=zlib size=134217728
backing_file=TEST_DIR/t.qcow2.1 backing_fmt=qcow2 lazy_refcounts=off
refcount_bits=16
{"return": {}}
=== Invalid command - missing device and nodename ===
-{ 'execute': 'blockdev-snapshot-sync', 'arguments': {
'snapshot-file':'TEST_DIR/1-snapshot-v0.IMGFMT', 'format': 'IMGFMT' } }
+{ 'execute': 'blockdev-snapshot-sync',
+ 'arguments': {
'snapshot-file':'TEST_DIR/1-snapshot-v0.IMGFMT',
+ 'format': 'IMGFMT' } }
{"error": {"class": "GenericError", "desc": "Cannot find device= nor
node_name="}}
=== Invalid command - missing snapshot-file ===
-{ 'execute': 'blockdev-snapshot-sync', 'arguments': { 'device': 'virtio0',
'format': 'IMGFMT' } }
+{ 'execute': 'blockdev-snapshot-sync',
+ 'arguments': { 'device': 'virtio0',
+ 'format': 'IMGFMT' } }
{"error": {"class": "GenericError", "desc": "Parameter 'snapshot-file' is
missing"}}
=== Create several transactional group snapshots ===
-{ 'execute': 'transaction', 'arguments': {'actions': [ { 'type':
'blockdev-snapshot-sync', 'data' : { 'device': 'virtio0', 'snapshot-file':
'TEST_DIR/2-snapshot-v0.IMGFMT' } }, { 'type': 'blockdev-snapshot-sync', 'data'
: { 'device': 'virtio1', 'snapshot-file': 'TEST_DIR/2-snapshot-v1.IMGFMT' } } ]
} }
+{ 'execute': 'transaction', 'arguments':
+ {'actions': [
+ { 'type': 'blockdev-snapshot-sync', 'data' :
+ { 'device': 'virtio0',
+ 'snapshot-file': 'TEST_DIR/2-snapshot-v0.IMGFMT' } },
+ { 'type': 'blockdev-snapshot-sync', 'data' :
+ { 'device': 'virtio1',
+
[PATCH] iotests/210: Fix reference output
Commit 8b1170012b1 has added a global maximum disk length for the block
layer, so the error message when creating an overly large disk has
changed.
Fixes: 8b1170012b1de6649c66ac1887f4df7e312abf3b
("block: introduce BDRV_MAX_LENGTH")
Signed-off-by: Max Reitz
---
tests/qemu-iotests/210.out | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/210.out b/tests/qemu-iotests/210.out
index a5e88e2a82..dc1a3c9786 100644
--- a/tests/qemu-iotests/210.out
+++ b/tests/qemu-iotests/210.out
@@ -182,7 +182,7 @@ Job failed: The requested file size is too large
=== Resize image with invalid sizes ===
{"execute": "block_resize", "arguments": {"node-name": "node1", "size":
9223372036854775296}}
-{"error": {"class": "GenericError", "desc": "The requested file size is too
large"}}
+{"error": {"class": "GenericError", "desc": "Required too big image size, it
must be not greater than 9223372035781033984"}}
{"execute": "block_resize", "arguments": {"node-name": "node1", "size":
9223372036854775808}}
{"error": {"class": "GenericError", "desc": "Invalid parameter type for
'size', expected: integer"}}
{"execute": "block_resize", "arguments": {"node-name": "node1", "size":
18446744073709551104}}
--
2.29.2
Re: [PATCH v4 23/32] qdev: Move dev->realized check to qdev_property_set()
On Mon, Dec 14, 2020 at 03:55:30PM +0100, Igor Mammedov wrote:
> On Fri, 11 Dec 2020 17:05:20 -0500
> Eduardo Habkost wrote:
>
> > Every single qdev property setter function manually checks
> > dev->realized. We can just check dev->realized inside
> > qdev_property_set() instead.
> >
> > The check is being added as a separate function
> > (qdev_prop_allow_set()) because it will become a callback later.
>
> is callback added within this series?
> and I'd add here what's the purpose of it.
It will be added in part 2 of the series. See v3:
https://lore.kernel.org/qemu-devel/20201112214350.872250-35-ehabk...@redhat.com/
I don't know what else I could say about its purpose, in addition
to what I wrote above, and the comment below[1].
If you are just curious about the callback and confused because
it is not anywhere in this series, I can just remove the
paragraph above from the commit message. Would that be enough?
>
[...]
> > +/* returns: true if property is allowed to be set, false otherwise */
[1] ^^^
> > +static bool qdev_prop_allow_set(Object *obj, const char *name,
> > +Error **errp)
> > +{
> > +DeviceState *dev = DEVICE(obj);
> > +
> > +if (dev->realized) {
> > +qdev_prop_set_after_realize(dev, name, errp);
> > +return false;
> > +}
> > +return true;
> > +}
> > +
--
Eduardo
[PATCH v2 4/4] block: Close block exports in two steps
There's a cross-dependency between closing the block exports and
draining the block layer. The latter needs that we close all export's
client connections to ensure they won't queue more requests, but the
exports may have coroutines yielding in the block layer, which implies
they can't be fully closed until we drain it.
To break this cross-dependency, this change adds a "bool wait"
argument to blk_exp_close_all() and blk_exp_close_all_type(), so
callers can decide whether they want to wait for the exports to be
fully quiesced, or just return after requesting them to shut down.
Then, in bdrv_close_all we make two calls, one without waiting to
close all client connections, and another after draining the block
layer, this time waiting for the exports to be fully quiesced.
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1900505
Signed-off-by: Sergio Lopez
---
block.c | 20 +++-
block/export/export.c | 10 ++
blockdev-nbd.c| 2 +-
include/block/export.h| 4 ++--
qemu-nbd.c| 2 +-
stubs/blk-exp-close-all.c | 2 +-
6 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/block.c b/block.c
index bc8a66ab6e..41db70ac07 100644
--- a/block.c
+++ b/block.c
@@ -4472,13 +4472,31 @@ static void bdrv_close(BlockDriverState *bs)
void bdrv_close_all(void)
{
assert(job_next(NULL) == NULL);
-blk_exp_close_all();
+
+/*
+ * There's a cross-dependency between closing the block exports and
+ * draining the block layer. The latter needs that we close all export's
+ * client connections to ensure they won't queue more requests, but the
+ * exports may have coroutines yielding in the block layer, which implies
+ * they can't be fully closed until we drain it.
+ *
+ * Make a first call to close all export's client connections, without
+ * waiting for each export to be fully quiesced.
+ */
+blk_exp_close_all(false);
/* Drop references from requests still in flight, such as canceled block
* jobs whose AIO context has not been polled yet */
bdrv_drain_all();
blk_remove_all_bs();
+
+/*
+ * Make a second call to shut down the exports, this time waiting for them
+ * to be fully quiesced.
+ */
+blk_exp_close_all(true);
+
blockdev_close_all_bdrv_states();
assert(QTAILQ_EMPTY(&all_bdrv_states));
diff --git a/block/export/export.c b/block/export/export.c
index bad6f21b1c..0124ebd9f9 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -280,7 +280,7 @@ static bool blk_exp_has_type(BlockExportType type)
}
/* type == BLOCK_EXPORT_TYPE__MAX for all types */
-void blk_exp_close_all_type(BlockExportType type)
+void blk_exp_close_all_type(BlockExportType type, bool wait)
{
BlockExport *exp, *next;
@@ -293,12 +293,14 @@ void blk_exp_close_all_type(BlockExportType type)
blk_exp_request_shutdown(exp);
}
-AIO_WAIT_WHILE(NULL, blk_exp_has_type(type));
+if (wait) {
+AIO_WAIT_WHILE(NULL, blk_exp_has_type(type));
+}
}
-void blk_exp_close_all(void)
+void blk_exp_close_all(bool wait)
{
-blk_exp_close_all_type(BLOCK_EXPORT_TYPE__MAX);
+blk_exp_close_all_type(BLOCK_EXPORT_TYPE__MAX, wait);
}
void qmp_block_export_add(BlockExportOptions *export, Error **errp)
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index d8443d235b..d71d4da7c2 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -266,7 +266,7 @@ void qmp_nbd_server_stop(Error **errp)
return;
}
-blk_exp_close_all_type(BLOCK_EXPORT_TYPE_NBD);
+blk_exp_close_all_type(BLOCK_EXPORT_TYPE_NBD, true);
nbd_server_free(nbd_server);
nbd_server = NULL;
diff --git a/include/block/export.h b/include/block/export.h
index 7feb02e10d..71c25928ce 100644
--- a/include/block/export.h
+++ b/include/block/export.h
@@ -83,7 +83,7 @@ BlockExport *blk_exp_find(const char *id);
void blk_exp_ref(BlockExport *exp);
void blk_exp_unref(BlockExport *exp);
void blk_exp_request_shutdown(BlockExport *exp);
-void blk_exp_close_all(void);
-void blk_exp_close_all_type(BlockExportType type);
+void blk_exp_close_all(bool wait);
+void blk_exp_close_all_type(BlockExportType type, bool wait);
#endif
diff --git a/qemu-nbd.c b/qemu-nbd.c
index a7075c5419..928f4466f6 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -1122,7 +1122,7 @@ int main(int argc, char **argv)
do {
main_loop_wait(false);
if (state == TERMINATE) {
-blk_exp_close_all();
+blk_exp_close_all(true);
state = TERMINATED;
}
} while (state != TERMINATED);
diff --git a/stubs/blk-exp-close-all.c b/stubs/blk-exp-close-all.c
index 1c71316763..ecd0ce611f 100644
--- a/stubs/blk-exp-close-all.c
+++ b/stubs/blk-exp-close-all.c
@@ -2,6 +2,6 @@
#include "block/export.h"
/* Only used in programs that support block exports (libblockdev.fa) */
-void blk_exp_close_all(void)
+void blk_exp_close_all(bool wait)
{
[PATCH v2 1/4] block: Honor blk_set_aio_context() context requirements
The documentation for bdrv_set_aio_context_ignore() states this:
* The caller must own the AioContext lock for the old AioContext of bs, but it
* must not own the AioContext lock for new_context (unless new_context is the
* same as the current context of bs).
As blk_set_aio_context() makes use of this function, this rule also
applies to it.
Fix all occurrences where this rule wasn't honored.
Suggested-by: Kevin Wolf
Signed-off-by: Sergio Lopez
---
hw/block/dataplane/virtio-blk.c | 4
hw/block/dataplane/xen-block.c | 7 ++-
hw/scsi/virtio-scsi.c | 6 --
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 37499c5564..e9050c8987 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -172,6 +172,7 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
VirtIOBlockDataPlane *s = vblk->dataplane;
BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vblk)));
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+AioContext *old_context;
unsigned i;
unsigned nvqs = s->conf->num_queues;
Error *local_err = NULL;
@@ -214,7 +215,10 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
vblk->dataplane_started = true;
trace_virtio_blk_data_plane_start(s);
+old_context = blk_get_aio_context(s->conf->conf.blk);
+aio_context_acquire(old_context);
r = blk_set_aio_context(s->conf->conf.blk, s->ctx, &local_err);
+aio_context_release(old_context);
if (r < 0) {
error_report_err(local_err);
goto fail_guest_notifiers;
diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c
index 71c337c7b7..3675f8deaf 100644
--- a/hw/block/dataplane/xen-block.c
+++ b/hw/block/dataplane/xen-block.c
@@ -725,6 +725,7 @@ void xen_block_dataplane_start(XenBlockDataPlane *dataplane,
{
ERRP_GUARD();
XenDevice *xendev = dataplane->xendev;
+AioContext *old_context;
unsigned int ring_size;
unsigned int i;
@@ -808,10 +809,14 @@ void xen_block_dataplane_start(XenBlockDataPlane
*dataplane,
goto stop;
}
-aio_context_acquire(dataplane->ctx);
+old_context = blk_get_aio_context(dataplane->blk);
+aio_context_acquire(old_context);
/* If other users keep the BlockBackend in the iothread, that's ok */
blk_set_aio_context(dataplane->blk, dataplane->ctx, NULL);
+aio_context_release(old_context);
+
/* Only reason for failure is a NULL channel */
+aio_context_acquire(dataplane->ctx);
xen_device_set_event_channel_context(xendev, dataplane->event_channel,
dataplane->ctx, &error_abort);
aio_context_release(dataplane->ctx);
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 3db9a8aae9..7a347ceac5 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -821,15 +821,17 @@ static void virtio_scsi_hotplug(HotplugHandler
*hotplug_dev, DeviceState *dev,
VirtIODevice *vdev = VIRTIO_DEVICE(hotplug_dev);
VirtIOSCSI *s = VIRTIO_SCSI(vdev);
SCSIDevice *sd = SCSI_DEVICE(dev);
+AioContext *old_context;
int ret;
if (s->ctx && !s->dataplane_fenced) {
if (blk_op_is_blocked(sd->conf.blk, BLOCK_OP_TYPE_DATAPLANE, errp)) {
return;
}
-virtio_scsi_acquire(s);
+old_context = blk_get_aio_context(sd->conf.blk);
+aio_context_acquire(old_context);
ret = blk_set_aio_context(sd->conf.blk, s->ctx, errp);
-virtio_scsi_release(s);
+aio_context_release(old_context);
if (ret < 0) {
return;
}
--
2.26.2
[PATCH v2 0/4] nbd/server: Quiesce coroutines on context switch
This series allows the NBD server to properly switch between AIO contexts, having quiesced recv_coroutine and send_coroutine before doing the transition. We need this because we send back devices running in IO Thread owned contexts to the main context when stopping the data plane, something that can happen multiple times during the lifetime of a VM (usually during the boot sequence or on a reboot), and we drag the NBD server of the correspoing export with it. While there, fix also a problem caused by a cross-dependency between closing the export's client connections and draining the block layer. The visible effect of this problem was QEMU getting hung when the guest request a power off while there's an active NBD client. v2: - Replace "virtio-blk: Acquire context while switching them on dataplane start" with "block: Honor blk_set_aio_context() context requirements" (Kevin Wolf) - Add "block: Avoid processing BDS twice in bdrv_set_aio_context_ignore()" - Add "block: Close block exports in two steps" - Rename nbd_read_eof() to nbd_server_read_eof() (Eric Blake) - Fix double space and typo in comment. (Eric Blake) Sergio Lopez (4): block: Honor blk_set_aio_context() context requirements block: Avoid processing BDS twice in bdrv_set_aio_context_ignore() nbd/server: Quiesce coroutines on context switch block: Close block exports in two steps block.c | 27 ++- block/export/export.c | 10 +-- blockdev-nbd.c | 2 +- hw/block/dataplane/virtio-blk.c | 4 ++ hw/block/dataplane/xen-block.c | 7 +- hw/scsi/virtio-scsi.c | 6 +- include/block/export.h | 4 +- nbd/server.c| 120 qemu-nbd.c | 2 +- stubs/blk-exp-close-all.c | 2 +- 10 files changed, 156 insertions(+), 28 deletions(-) -- 2.26.2
[PATCH v2 3/4] nbd/server: Quiesce coroutines on context switch
When switching between AIO contexts we need to me make sure that both
recv_coroutine and send_coroutine are not scheduled to run. Otherwise,
QEMU may crash while attaching the new context with an error like
this one:
aio_co_schedule: Co-routine was already scheduled in 'aio_co_schedule'
To achieve this we need a local implementation of
'qio_channel_readv_all_eof' named 'nbd_read_eof' (a trick already done
by 'nbd/client.c') that allows us to interrupt the operation and to
know when recv_coroutine is yielding.
With this in place, we delegate detaching the AIO context to the
owning context with a BH ('nbd_aio_detach_bh') scheduled using
'aio_wait_bh_oneshot'. This BH signals that we need to quiesce the
channel by setting 'client->quiescing' to 'true', and either waits for
the coroutine to finish using AIO_WAIT_WHILE or, if it's yielding in
'nbd_read_eof', actively enters the coroutine to interrupt it.
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1900326
Signed-off-by: Sergio Lopez
Reviewed-by: Eric Blake
---
nbd/server.c | 120 +--
1 file changed, 106 insertions(+), 14 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 613ed2634a..7229f487d2 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -132,6 +132,9 @@ struct NBDClient {
CoMutex send_lock;
Coroutine *send_coroutine;
+bool read_yielding;
+bool quiescing;
+
QTAILQ_ENTRY(NBDClient) next;
int nb_requests;
bool closing;
@@ -1352,14 +1355,60 @@ static coroutine_fn int nbd_negotiate(NBDClient
*client, Error **errp)
return 0;
}
-static int nbd_receive_request(QIOChannel *ioc, NBDRequest *request,
+/* nbd_read_eof
+ * Tries to read @size bytes from @ioc. This is a local implementation of
+ * qio_channel_readv_all_eof. We have it here because we need it to be
+ * interruptible and to know when the coroutine is yielding.
+ * Returns 1 on success
+ * 0 on eof, when no data was read (errp is not set)
+ * negative errno on failure (errp is set)
+ */
+static inline int coroutine_fn
+nbd_read_eof(NBDClient *client, void *buffer, size_t size, Error **errp)
+{
+bool partial = false;
+
+assert(size);
+while (size > 0) {
+struct iovec iov = { .iov_base = buffer, .iov_len = size };
+ssize_t len;
+
+len = qio_channel_readv(client->ioc, &iov, 1, errp);
+if (len == QIO_CHANNEL_ERR_BLOCK) {
+client->read_yielding = true;
+qio_channel_yield(client->ioc, G_IO_IN);
+client->read_yielding = false;
+if (client->quiescing) {
+return -EAGAIN;
+}
+continue;
+} else if (len < 0) {
+return -EIO;
+} else if (len == 0) {
+if (partial) {
+error_setg(errp,
+ "Unexpected end-of-file before all bytes were
read");
+return -EIO;
+} else {
+return 0;
+}
+}
+
+partial = true;
+size -= len;
+buffer = (uint8_t *) buffer + len;
+}
+return 1;
+}
+
+static int nbd_receive_request(NBDClient *client, NBDRequest *request,
Error **errp)
{
uint8_t buf[NBD_REQUEST_SIZE];
uint32_t magic;
int ret;
-ret = nbd_read(ioc, buf, sizeof(buf), "request", errp);
+ret = nbd_read_eof(client, buf, sizeof(buf), errp);
if (ret < 0) {
return ret;
}
@@ -1480,11 +1529,37 @@ static void blk_aio_attached(AioContext *ctx, void
*opaque)
QTAILQ_FOREACH(client, &exp->clients, next) {
qio_channel_attach_aio_context(client->ioc, ctx);
+
+assert(client->recv_coroutine == NULL);
+assert(client->send_coroutine == NULL);
+
+if (client->quiescing) {
+client->quiescing = false;
+nbd_client_receive_next_request(client);
+}
+}
+}
+
+static void nbd_aio_detach_bh(void *opaque)
+{
+NBDExport *exp = opaque;
+NBDClient *client;
+
+QTAILQ_FOREACH(client, &exp->clients, next) {
+qio_channel_detach_aio_context(client->ioc);
+client->quiescing = true;
+
if (client->recv_coroutine) {
-aio_co_schedule(ctx, client->recv_coroutine);
+if (client->read_yielding) {
+qemu_aio_coroutine_enter(exp->common.ctx,
+ client->recv_coroutine);
+} else {
+AIO_WAIT_WHILE(exp->common.ctx, client->recv_coroutine !=
NULL);
+}
}
+
if (client->send_coroutine) {
-aio_co_schedule(ctx, client->send_coroutine);
+AIO_WAIT_WHILE(exp->common.ctx, client->send_coroutine != NULL);
}
}
}
@@ -1492,13 +1567,10 @@ static void blk_aio_attached(AioContext *ctx, void
*opaque)
static void blk_aio_detach(void *opaque)
{
NBDExport *exp = opaque;
-NBDClient *client;
trace_nbd_blk_ai
[PATCH v2 2/4] block: Avoid processing BDS twice in bdrv_set_aio_context_ignore()
While processing the parents of a BDS, one of the parents may process
the child that's doing the tail recursion, which leads to a BDS being
processed twice. This is especially problematic for the aio_notifiers,
as they might attempt to work on both the old and the new AIO
contexts.
To avoid this, add the BDS pointer to the ignore list, and check the
child BDS pointer while iterating over the children.
Signed-off-by: Sergio Lopez
---
block.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index f1cedac362..bc8a66ab6e 100644
--- a/block.c
+++ b/block.c
@@ -6465,12 +6465,17 @@ void bdrv_set_aio_context_ignore(BlockDriverState *bs,
bdrv_drained_begin(bs);
QLIST_FOREACH(child, &bs->children, next) {
-if (g_slist_find(*ignore, child)) {
+if (g_slist_find(*ignore, child) || g_slist_find(*ignore, child->bs)) {
continue;
}
*ignore = g_slist_prepend(*ignore, child);
bdrv_set_aio_context_ignore(child->bs, new_context, ignore);
}
+/*
+ * Add a reference to this BS to the ignore list, so its
+ * parents won't attempt to process it again.
+ */
+*ignore = g_slist_prepend(*ignore, bs);
QLIST_FOREACH(child, &bs->parents, next_parent) {
if (g_slist_find(*ignore, child)) {
continue;
--
2.26.2
Re: [PATCH v1 0/2] Add timeout mechanism to qmp actions
On Tue, Dec 08, 2020 at 08:47:42AM -0500, Glauber Costa wrote: > The work we did at the time was in fixing those things in the kernel > as much as we could. > But the API is just like that... Thanks! Stefan signature.asc Description: PGP signature
Re: [PATCH v2 04/36] block: bdrv_append(): return status
On Fri 27 Nov 2020 03:44:50 PM CET, Vladimir Sementsov-Ogievskiy wrote: > Return int status to avoid extra error propagation schemes. > > Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Alberto Garcia Berto
Re: [PATCH v4 30/32] qdev: Rename qdev_get_prop_ptr() to object_field_prop_ptr()
On Fri, 11 Dec 2020 17:05:27 -0500
Eduardo Habkost wrote:
> The function will be moved to common QOM code, as it is not
> specific to TYPE_DEVICE anymore.
>
> Reviewed-by: Stefan Berger
> Signed-off-by: Eduardo Habkost
Reviewed-by: Igor Mammedov
> ---
> Changes v1 -> v2:
> * Rename to object_field_prop_ptr() instead of object_static_prop_ptr()
> ---
> Cc: Stefan Berger
> Cc: Stefano Stabellini
> Cc: Anthony Perard
> Cc: Paul Durrant
> Cc: Kevin Wolf
> Cc: Max Reitz
> Cc: Paolo Bonzini
> Cc: "Daniel P. Berrangé"
> Cc: Eduardo Habkost
> Cc: Cornelia Huck
> Cc: Halil Pasic
> Cc: Christian Borntraeger
> Cc: Richard Henderson
> Cc: David Hildenbrand
> Cc: Thomas Huth
> Cc: Matthew Rosato
> Cc: Alex Williamson
> Cc: qemu-de...@nongnu.org
> Cc: xen-de...@lists.xenproject.org
> Cc: qemu-block@nongnu.org
> Cc: qemu-s3...@nongnu.org
> ---
> include/hw/qdev-properties.h | 2 +-
> backends/tpm/tpm_util.c | 6 ++--
> hw/block/xen-block.c | 4 +--
> hw/core/qdev-properties-system.c | 50 +-
> hw/core/qdev-properties.c| 60
> hw/s390x/css.c | 4 +--
> hw/s390x/s390-pci-bus.c | 4 +--
> hw/vfio/pci-quirks.c | 4 +--
> 8 files changed, 67 insertions(+), 67 deletions(-)
>
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index 90222822f1..97bb9494ae 100644
> --- a/include/hw/qdev-properties.h
> +++ b/include/hw/qdev-properties.h
> @@ -193,7 +193,7 @@ void qdev_prop_set_macaddr(DeviceState *dev, const char
> *name,
> const uint8_t *value);
> void qdev_prop_set_enum(DeviceState *dev, const char *name, int value);
>
> -void *qdev_get_prop_ptr(Object *obj, Property *prop);
> +void *object_field_prop_ptr(Object *obj, Property *prop);
>
> void qdev_prop_register_global(GlobalProperty *prop);
> const GlobalProperty *qdev_find_global_prop(Object *obj,
> diff --git a/backends/tpm/tpm_util.c b/backends/tpm/tpm_util.c
> index 39b45fa46d..a6e6d3e72f 100644
> --- a/backends/tpm/tpm_util.c
> +++ b/backends/tpm/tpm_util.c
> @@ -35,7 +35,7 @@
> static void get_tpm(Object *obj, Visitor *v, const char *name, void *opaque,
> Error **errp)
> {
> -TPMBackend **be = qdev_get_prop_ptr(obj, opaque);
> +TPMBackend **be = object_field_prop_ptr(obj, opaque);
> char *p;
>
> p = g_strdup(*be ? (*be)->id : "");
> @@ -47,7 +47,7 @@ static void set_tpm(Object *obj, Visitor *v, const char
> *name, void *opaque,
> Error **errp)
> {
> Property *prop = opaque;
> -TPMBackend *s, **be = qdev_get_prop_ptr(obj, prop);
> +TPMBackend *s, **be = object_field_prop_ptr(obj, prop);
> char *str;
>
> if (!visit_type_str(v, name, &str, errp)) {
> @@ -67,7 +67,7 @@ static void set_tpm(Object *obj, Visitor *v, const char
> *name, void *opaque,
> static void release_tpm(Object *obj, const char *name, void *opaque)
> {
> Property *prop = opaque;
> -TPMBackend **be = qdev_get_prop_ptr(obj, prop);
> +TPMBackend **be = object_field_prop_ptr(obj, prop);
>
> if (*be) {
> tpm_backend_reset(*be);
> diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
> index bd1aef63a7..718d886e5c 100644
> --- a/hw/block/xen-block.c
> +++ b/hw/block/xen-block.c
> @@ -336,7 +336,7 @@ static void xen_block_get_vdev(Object *obj, Visitor *v,
> const char *name,
> void *opaque, Error **errp)
> {
> Property *prop = opaque;
> -XenBlockVdev *vdev = qdev_get_prop_ptr(obj, prop);
> +XenBlockVdev *vdev = object_field_prop_ptr(obj, prop);
> char *str;
>
> switch (vdev->type) {
> @@ -396,7 +396,7 @@ static void xen_block_set_vdev(Object *obj, Visitor *v,
> const char *name,
> void *opaque, Error **errp)
> {
> Property *prop = opaque;
> -XenBlockVdev *vdev = qdev_get_prop_ptr(obj, prop);
> +XenBlockVdev *vdev = object_field_prop_ptr(obj, prop);
> char *str, *p;
> const char *end;
>
> diff --git a/hw/core/qdev-properties-system.c
> b/hw/core/qdev-properties-system.c
> index 590c5f3d97..e6d378a34e 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -62,7 +62,7 @@ static void get_drive(Object *obj, Visitor *v, const char
> *name, void *opaque,
>Error **errp)
> {
> Property *prop = opaque;
> -void **ptr = qdev_get_prop_ptr(obj, prop);
> +void **ptr = object_field_prop_ptr(obj, prop);
> const char *value;
> char *p;
>
> @@ -88,7 +88,7 @@ static void set_drive_helper(Object *obj, Visitor *v, const
> char *name,
> {
> DeviceState *dev = DEVICE(obj);
> Property *prop = opaque;
> -void **ptr = qdev_get_prop_ptr(obj, prop);
> +void **ptr = object_field_prop_ptr(obj, prop);
> char *str;
> BlockBackend *blk;
> bool blk_created = false;
>
Re: [PATCH v4 23/32] qdev: Move dev->realized check to qdev_property_set()
On Fri, 11 Dec 2020 17:05:20 -0500
Eduardo Habkost wrote:
> Every single qdev property setter function manually checks
> dev->realized. We can just check dev->realized inside
> qdev_property_set() instead.
>
> The check is being added as a separate function
> (qdev_prop_allow_set()) because it will become a callback later.
is callback added within this series?
and I'd add here what's the purpose of it.
>
> Reviewed-by: Stefan Berger
> Signed-off-by: Eduardo Habkost
> ---
> Changes v1 -> v2:
> * Removed unused variable at xen_block_set_vdev()
> * Redone patch after changes in the previous patches in the
> series
> ---
> Cc: Stefan Berger
> Cc: Stefano Stabellini
> Cc: Anthony Perard
> Cc: Paul Durrant
> Cc: Kevin Wolf
> Cc: Max Reitz
> Cc: Paolo Bonzini
> Cc: "Daniel P. Berrangé"
> Cc: Eduardo Habkost
> Cc: Cornelia Huck
> Cc: Halil Pasic
> Cc: Christian Borntraeger
> Cc: Richard Henderson
> Cc: David Hildenbrand
> Cc: Thomas Huth
> Cc: Matthew Rosato
> Cc: Alex Williamson
> Cc: Mark Cave-Ayland
> Cc: Artyom Tarasenko
> Cc: qemu-de...@nongnu.org
> Cc: xen-de...@lists.xenproject.org
> Cc: qemu-block@nongnu.org
> Cc: qemu-s3...@nongnu.org
> ---
> backends/tpm/tpm_util.c | 6 --
> hw/block/xen-block.c | 6 --
> hw/core/qdev-properties-system.c | 70 --
> hw/core/qdev-properties.c| 100 ++-
> hw/s390x/css.c | 6 --
> hw/s390x/s390-pci-bus.c | 6 --
> hw/vfio/pci-quirks.c | 6 --
> target/sparc/cpu.c | 6 --
> 8 files changed, 18 insertions(+), 188 deletions(-)
>
> diff --git a/backends/tpm/tpm_util.c b/backends/tpm/tpm_util.c
> index a5d997e7dc..39b45fa46d 100644
> --- a/backends/tpm/tpm_util.c
> +++ b/backends/tpm/tpm_util.c
> @@ -46,16 +46,10 @@ static void get_tpm(Object *obj, Visitor *v, const char
> *name, void *opaque,
> static void set_tpm(Object *obj, Visitor *v, const char *name, void *opaque,
> Error **errp)
> {
> -DeviceState *dev = DEVICE(obj);
> Property *prop = opaque;
> TPMBackend *s, **be = qdev_get_prop_ptr(obj, prop);
> char *str;
>
> -if (dev->realized) {
> -qdev_prop_set_after_realize(dev, name, errp);
> -return;
> -}
> -
> if (!visit_type_str(v, name, &str, errp)) {
> return;
> }
> diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
> index 905e4acd97..bd1aef63a7 100644
> --- a/hw/block/xen-block.c
> +++ b/hw/block/xen-block.c
> @@ -395,17 +395,11 @@ static int vbd_name_to_disk(const char *name, const
> char **endp,
> static void xen_block_set_vdev(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> -DeviceState *dev = DEVICE(obj);
> Property *prop = opaque;
> XenBlockVdev *vdev = qdev_get_prop_ptr(obj, prop);
> char *str, *p;
> const char *end;
>
> -if (dev->realized) {
> -qdev_prop_set_after_realize(dev, name, errp);
> -return;
> -}
> -
> if (!visit_type_str(v, name, &str, errp)) {
> return;
> }
> diff --git a/hw/core/qdev-properties-system.c
> b/hw/core/qdev-properties-system.c
> index 42529c3b65..f31aea3de1 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -94,11 +94,6 @@ static void set_drive_helper(Object *obj, Visitor *v,
> const char *name,
> bool blk_created = false;
> int ret;
>
> -if (dev->realized) {
> -qdev_prop_set_after_realize(dev, name, errp);
> -return;
> -}
> -
> if (!visit_type_str(v, name, &str, errp)) {
> return;
> }
> @@ -230,17 +225,11 @@ static void get_chr(Object *obj, Visitor *v, const char
> *name, void *opaque,
> static void set_chr(Object *obj, Visitor *v, const char *name, void *opaque,
> Error **errp)
> {
> -DeviceState *dev = DEVICE(obj);
> Property *prop = opaque;
> CharBackend *be = qdev_get_prop_ptr(obj, prop);
> Chardev *s;
> char *str;
>
> -if (dev->realized) {
> -qdev_prop_set_after_realize(dev, name, errp);
> -return;
> -}
> -
> if (!visit_type_str(v, name, &str, errp)) {
> return;
> }
> @@ -311,18 +300,12 @@ static void get_mac(Object *obj, Visitor *v, const char
> *name, void *opaque,
> static void set_mac(Object *obj, Visitor *v, const char *name, void *opaque,
> Error **errp)
> {
> -DeviceState *dev = DEVICE(obj);
> Property *prop = opaque;
> MACAddr *mac = qdev_get_prop_ptr(obj, prop);
> int i, pos;
> char *str;
> const char *p;
>
> -if (dev->realized) {
> -qdev_prop_set_after_realize(dev, name, errp);
> -return;
> -}
> -
> if (!visit_type_str(v, name, &str, errp)) {
> return;
> }
> @@ -390,7 +373,6 @@ static void get_netdev(Object *obj, Visitor *v, const
> char *name
Re: [PATCH v4 00/16] 64bit block-layer: part I
11.12.2020 21:39, Vladimir Sementsov-Ogievskiy wrote: Hi all! We want 64bit write-zeroes, and for this, convert all io functions to 64bit. We chose signed type, to be consistent with off_t (which is signed) and with possibility for signed return type (where negative value means error). Please refer to initial cover-letter https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg08723.html for more info. v4: I found, that some more work is needed for block/block-backend, so decided to make partI, converting block/io v4 is based on Kevin's block branch ([PULL 00/34] Block layer patches) for BDRV_MAX_LENGTH changes: 01-05: new 06: add Alberto's r-b 07: new 08-16: rebase, add new-style request check, improve commit-msg, drop r-bs Based-on:<20201211170812.228643-1-kw...@redhat.com> Now based on master. -- Best regards, Vladimir
Re: [PATCH v4 30/32] qdev: Rename qdev_get_prop_ptr() to object_field_prop_ptr()
On Fri, 11 Dec 2020 17:05:27 -0500 Eduardo Habkost wrote: > The function will be moved to common QOM code, as it is not > specific to TYPE_DEVICE anymore. > > Reviewed-by: Stefan Berger > Signed-off-by: Eduardo Habkost > --- > Changes v1 -> v2: > * Rename to object_field_prop_ptr() instead of object_static_prop_ptr() > --- > Cc: Stefan Berger > Cc: Stefano Stabellini > Cc: Anthony Perard > Cc: Paul Durrant > Cc: Kevin Wolf > Cc: Max Reitz > Cc: Paolo Bonzini > Cc: "Daniel P. Berrangé" > Cc: Eduardo Habkost > Cc: Cornelia Huck > Cc: Halil Pasic > Cc: Christian Borntraeger > Cc: Richard Henderson > Cc: David Hildenbrand > Cc: Thomas Huth > Cc: Matthew Rosato > Cc: Alex Williamson > Cc: qemu-de...@nongnu.org > Cc: xen-de...@lists.xenproject.org > Cc: qemu-block@nongnu.org > Cc: qemu-s3...@nongnu.org > --- > include/hw/qdev-properties.h | 2 +- > backends/tpm/tpm_util.c | 6 ++-- > hw/block/xen-block.c | 4 +-- > hw/core/qdev-properties-system.c | 50 +- > hw/core/qdev-properties.c| 60 > hw/s390x/css.c | 4 +-- > hw/s390x/s390-pci-bus.c | 4 +-- > hw/vfio/pci-quirks.c | 4 +-- > 8 files changed, 67 insertions(+), 67 deletions(-) Reviewed-by: Cornelia Huck
Re: [PATCH v4 23/32] qdev: Move dev->realized check to qdev_property_set()
On Fri, 11 Dec 2020 17:05:20 -0500 Eduardo Habkost wrote: > Every single qdev property setter function manually checks > dev->realized. We can just check dev->realized inside > qdev_property_set() instead. > > The check is being added as a separate function > (qdev_prop_allow_set()) because it will become a callback later. > > Reviewed-by: Stefan Berger > Signed-off-by: Eduardo Habkost > --- > Changes v1 -> v2: > * Removed unused variable at xen_block_set_vdev() > * Redone patch after changes in the previous patches in the > series > --- > Cc: Stefan Berger > Cc: Stefano Stabellini > Cc: Anthony Perard > Cc: Paul Durrant > Cc: Kevin Wolf > Cc: Max Reitz > Cc: Paolo Bonzini > Cc: "Daniel P. Berrangé" > Cc: Eduardo Habkost > Cc: Cornelia Huck > Cc: Halil Pasic > Cc: Christian Borntraeger > Cc: Richard Henderson > Cc: David Hildenbrand > Cc: Thomas Huth > Cc: Matthew Rosato > Cc: Alex Williamson > Cc: Mark Cave-Ayland > Cc: Artyom Tarasenko > Cc: qemu-de...@nongnu.org > Cc: xen-de...@lists.xenproject.org > Cc: qemu-block@nongnu.org > Cc: qemu-s3...@nongnu.org > --- > backends/tpm/tpm_util.c | 6 -- > hw/block/xen-block.c | 6 -- > hw/core/qdev-properties-system.c | 70 -- > hw/core/qdev-properties.c| 100 ++- > hw/s390x/css.c | 6 -- > hw/s390x/s390-pci-bus.c | 6 -- > hw/vfio/pci-quirks.c | 6 -- > target/sparc/cpu.c | 6 -- > 8 files changed, 18 insertions(+), 188 deletions(-) Reviewed-by: Cornelia Huck
Re: [PATCH v3] hw/block/nand: Decommission the NAND museum
On Mon, 14 Dec 2020 at 00:26, Philippe Mathieu-Daudé wrote: > > This is the QEMU equivalent of this Linux commit (but 7 years later): > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f7025a43a9da2 > > The MTD subsystem has its own small museum of ancient NANDs > in a form of the CONFIG_MTD_NAND_MUSEUM_IDS configuration option. > The museum contains stone age NANDs with 256 bytes pages, as well > as iron age NANDs with 512 bytes per page and up to 8MiB page size. > > It is with great sorrow that I inform you that the museum is being > decommissioned. The MTD subsystem is out of budget for Kconfig > options and already has too many of them, and there is a general > kernel trend to simplify the configuration menu. > > We remove the stone age exhibits along with closing the museum, > but some of the iron age ones are transferred to the regular NAND > depot. Namely, only those which have unique device IDs are > transferred, and the ones which have conflicting device IDs are > removed. > > The machine using this device are: > - axis-dev88 > - tosa (via tc6393xb_init) > - spitz based (akita, borzoi, terrier) > > Reviewed-by: Richard Henderson > Signed-off-by: Philippe Mathieu-Daudé > --- > v3: Do not manually convert tabs to space to avoid mistakes... Reviewed-by: Peter Maydell thanks -- PMM
