On Thu, Oct 05, 2023 at 04:55:50PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> In many cases we just want an effect of qmp command and want to raise on
> failure. Use vm.cmd() method which does exactly this.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> tests/avocado/vnc.py
On Thu, Oct 05, 2023 at 04:55:49PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> We don't expect failure here and need 'result' object. cmd() is better
> in this case.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> tests/vm/basevm.py | 4 ++--
> 1 file changed, 2 insertions(+), 2
On Thu, Oct 05, 2023 at 04:55:48PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> The returned value is unused. It's simple to check by command
>
> git grep -B 3 '\.pause_job('
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> tests/qemu-iotests/iotests.py | 3 +--
> 1 file changed, 1
On Thu, Oct 05, 2023 at 04:55:47PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> qmp() method supports passing dict (if it doesn't need substituting
> '_' to '-' in keys). So, drop some extra '**' operators.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> tests/qemu-iotests/040
On Thu, Oct 05, 2023 at 04:55:46PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> tests/qemu-iotests/041 | 2 +-
> tests/qemu-iotests/196 | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
Maybe in the subject s/occasional/extra/
On Thu, Oct 05, 2023 at 04:55:45PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> To simplify further conversion.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> tests/qemu-iotests/040 | 3 ++-
> tests/qemu-iotests/147 | 3 ++-
> tests/qemu-iotests/155 | 4 ++--
> tests/qemu-iotests/218 |
On Thu, Oct 05, 2023 at 04:55:44PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> tests/qemu-iotests/041| 1 +
> tests/qemu-iotests/151| 1 +
> tests/qemu-iotests/152| 2 ++
>
On 11.09.23 12:46, Kevin Wolf wrote:
When the permission related BlockDriver callbacks are called, we are in
the middle of an operation traversing the block graph. Polling in such a
place is a very bad idea because the graph could change in unexpected
ways. In the future, callers will also hold
On Thu, Oct 05, 2023 at 04:55:43PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Add similar method for consistency.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> tests/qemu-iotests/iotests.py | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
Reviewed-by: Eric Blake
On Thu, Oct 05, 2023 at 04:55:42PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> The method is not popular in iotests, we prefer use vm.qmp() and then
> check success by hand.. But that's not optimal. To simplify movement to
extra '.'
> vm.cmd() let's support same interface improvements like in
On Thu, Oct 05, 2023 at 04:55:41PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Use a shorter name. We are going to move in iotests from qmp() to
> command() where possible. But command() is longer than qmp() and don't
> look better. Let's rename.
>
> You can simply grep for '\.command(' and for
On Thu, Oct 05, 2023 at 04:55:40PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Having cmd() and command() methods in one class doesn't look good.
> Rename cmd() to cmd_raw(), to show its meaning better.
>
> We also want to rename command() to cmd() in future, so this commit is
> a necessary
On Thu, Oct 05, 2023 at 04:55:39PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Here we don't expect a failure. In case on failure we'll crash on
s/case on/case of/
> trying to access ['return']. Let's better use .command() that clearly
> raise on failure.
Maybe: s/Let's better /Better is to/;
Allow a client to request a subset of negotiated meta contexts. For
example, a client may ask to use a single connection to learn about
both block status and dirty bitmaps, but where the dirty bitmap
queries only need to be performed on a subset of the disk; forcing the
server to compute that
Although extended mode is not yet enabled, once we do turn it on, we
need to reply with extended headers to all messages. Update the low
level entry points necessary so that all other callers automatically
get the right header based on the current mode.
Signed-off-by: Eric Blake
Reviewed-by:
Although extended mode is not yet enabled, once we do turn it on, we
need to accept extended requests for all messages. Previous patches
have already taken care of supporting 64-bit lengths, now we just need
to read it off the wire.
Note that this implementation will block indefinitely on a
Once extended mode is enabled, we need to accept 64-bit status replies
(even for replies that don't exceed a 32-bit length). It is easier to
normalize narrow replies into wide format so that the rest of our code
only has to handle one width. Although a server is non-compliant if
it sends a
Instead of ignoring the low-level error just to refabricate our own
message to pass to the caller, we can just plumb the caller's errp
down to the low level.
Signed-off-by: Eric Blake
Message-ID: <20230925192229.3186470-20-ebl...@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
All the pieces are in place for a client to finally request extended
headers. Note that we must not request extended headers when qemu-nbd
is used to connect to the kernel module (as nbd.ko does not expect
them, but expects us to do the negotiation in userspace before handing
the socket over to
The next commit will add support for the optional extension
NBD_CMD_FLAG_PAYLOAD during NBD_CMD_BLOCK_STATUS, where the client can
request that the server only return a subset of negotiated contexts,
rather than all contexts. To make that task easier, this patch
populates the list of contexts to
The NBD spec states that if the client negotiates extended headers,
the server must avoid NBD_REPLY_TYPE_BLOCK_STATUS and instead use
NBD_REPLY_TYPE_BLOCK_STATUS_EXT which supports 64-bit lengths, even if
the reply does not need more than 32 bits. As of this patch,
client->mode is still never
Peform several minor refactorings of how the list of negotiated meta
contexts is managed, to make upcoming patches easier: Promote the
internal type NBDExportMetaContexts to the public opaque type
NBDMetaContexts, and mark exp const. Use a shorter member name in
NBDClient. Hoist calls to
Upcoming additions to support NBD 64-bit effect lengths allow for the
possibility to distinguish between payload length (capped at 32M) and
effect length (64 bits, although we generally assume 63 bits because
of off_t limitations). Without that extension, only the NBD_CMD_WRITE
request has a
Update the client code to be able to send an extended request, and
parse an extended header from the server. Note that since we reject
any structured reply with a too-large payload, we can always normalize
a valid header back into the compact form, so that the caller need not
deal with two
Time to start supporting clients that request extended headers. Now
we can finally reach the code added across several previous patches.
Even though the NBD spec has been altered to allow us to accept
NBD_CMD_READ larger than the max payload size (provided our response
is a hole or broken up
On 05.10.23 18:38, Eric Blake wrote:
On Mon, Sep 25, 2023 at 02:22:31PM -0500, Eric Blake wrote:
Upcoming additions to support NBD 64-bit effect lengths allow for the
possibility to distinguish between payload length (capped at 32M) and
effect length (64 bits, although we generally assume 63
On Wed, Oct 4, 2023 at 8:00 AM Philippe Mathieu-Daudé wrote:
>
> Fix:
>
> hw/ide/ahci.c:1577:23: error: declaration shadows a local variable
> [-Werror,-Wshadow]
> IDEState *s = >port.ifs[j];
> ^
> hw/ide/ahci.c:1569:29: note: previous declaration is here
>
On Mon, Sep 25, 2023 at 02:22:31PM -0500, Eric Blake wrote:
> Upcoming additions to support NBD 64-bit effect lengths allow for the
> possibility to distinguish between payload length (capped at 32M) and
> effect length (64 bits, although we generally assume 63 bits because
> of off_t
On Thu, Oct 05, 2023 at 05:33:26PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > +static int
> > +nbd_co_block_status_payload_read(NBDClient *client, NBDRequest *request,
> > + Error **errp)
> > +{
> > +int payload_len = request->len;
>
> payload_len should be
On Thu, Oct 5, 2023, 9:00 AM Stefan Hajnoczi wrote:
> On Thu, 5 Oct 2023 at 00:49, Philippe Mathieu-Daudé
> wrote:
> >
> > Hi John,
> >
> > On 4/10/23 21:46, John Snow wrote:
> > > The following changes since commit
> da1034094d375afe9e3d8ec8980550ea0f06f7e0:
> > >
> > >Merge tag
On Thu, Oct 5, 2023, 10:05 AM Eric Blake wrote:
> On Wed, Oct 04, 2023 at 03:46:05PM -0400, John Snow wrote:
> > The test bails gracefully if this module isn't installed, but linters
> > need a little help understanding that. It's enough to just declare the
> > type in this case.
> >
> > (Fixes
Vladimir Sementsov-Ogievskiy wrote:
> On 26.07.23 16:32, Thomas Huth wrote:
>> On 26/07/2023 15.00, Peter Maydell wrote:
>>> On Wed, 26 Jul 2023 at 13:06, Juan Quintela wrote:
To make things easier, this is the part that show how it breaks (this is
the gcov test):
357/423
On 25.09.23 22:22, Eric Blake wrote:
Allow a client to request a subset of negotiated meta contexts. For
example, a client may ask to use a single connection to learn about
both block status and dirty bitmaps, but where the dirty bitmap
queries only need to be performed on a subset of the disk;
On 05.10.23 16:49, Eric Blake wrote:
On Wed, Oct 04, 2023 at 04:55:02PM -0500, Eric Blake wrote:
+static int
+nbd_co_block_status_payload_read(NBDClient *client, NBDRequest *request,
+ Error **errp)
[..]
+for (i = 0; i < count; i++) {
+id =
On Thu, Oct 05, 2023 at 04:55:38PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> We just want to ignore failure, so we don't need low level .cmd(). This
> helps further renaming .command() to .cmd().
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> python/qemu/qmp/qmp_shell.py | 20
On Thu, Oct 05, 2023 at 04:55:37PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> The argument is unused, let's drop it for now, as we are going to
> refactor the interface and don't want to refactor unused things.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> python/qemu/qmp/legacy.py
On Wed, Oct 04, 2023 at 03:46:05PM -0400, John Snow wrote:
> The test bails gracefully if this module isn't installed, but linters
> need a little help understanding that. It's enough to just declare the
> type in this case.
>
> (Fixes pylint complaining about use of an uninitialized variable
Use a shorter name. We are going to move in iotests from qmp() to
command() where possible. But command() is longer than qmp() and don't
look better. Let's rename.
You can simply grep for '\.command(' and for 'def command(' to check
that everything is updated (command() in tests/docker/docker.py
Having cmd() and command() methods in one class doesn't look good.
Rename cmd() to cmd_raw(), to show its meaning better.
We also want to rename command() to cmd() in future, so this commit is
a necessary step.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
python/qemu/machine/machine.py | 2
The argument is unused, let's drop it for now, as we are going to
refactor the interface and don't want to refactor unused things.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
python/qemu/qmp/legacy.py | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git
We don't expect failure here and need 'result' object. cmd() is better
in this case.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/vm/basevm.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index a97e23b0ce..8aef4cff96
To simplify further conversion.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/qemu-iotests/040 | 3 ++-
tests/qemu-iotests/147 | 3 ++-
tests/qemu-iotests/155 | 4 ++--
tests/qemu-iotests/218 | 4 ++--
tests/qemu-iotests/296 | 3 ++-
5 files changed, 10 insertions(+), 7 deletions(-)
Add similar method for consistency.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/qemu-iotests/iotests.py | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 8ffd9fb660..6cc50f0b50 100644
---
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/qemu-iotests/041| 1 +
tests/qemu-iotests/151| 1 +
tests/qemu-iotests/152| 2 ++
tests/qemu-iotests/tests/migrate-bitmaps-test | 2 ++
4 files changed, 6 insertions(+)
The returned value is unused. It's simple to check by command
git grep -B 3 '\.pause_job('
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/qemu-iotests/iotests.py | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/iotests.py
The method is not popular in iotests, we prefer use vm.qmp() and then
check success by hand.. But that's not optimal. To simplify movement to
vm.cmd() let's support same interface improvements like in vm.qmp().
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
python/qemu/machine/machine.py | 12
qmp() method supports passing dict (if it doesn't need substituting
'_' to '-' in keys). So, drop some extra '**' operators.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/qemu-iotests/040| 4 +-
tests/qemu-iotests/041| 14 +++---
Here we don't expect a failure. In case on failure we'll crash on
trying to access ['return']. Let's better use .command() that clearly
raise on failure.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
scripts/cpu-x86-uarch-abi.py | 12 ++--
1 file changed, 6 insertions(+), 6
We just want to ignore failure, so we don't need low level .cmd(). This
helps further renaming .command() to .cmd().
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
python/qemu/qmp/qmp_shell.py | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/qemu-iotests/041 | 2 +-
tests/qemu-iotests/196 | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index 4d7a829b65..550e4dc391 100755
--- a/tests/qemu-iotests/041
+++
Hi all!
Let's get rid of pattern
result = self.vm.qmp(...)
self.assert_qmp(result, 'return', {})
And switch to just
self.vm.cmd(...)
v6: rebase on master
Vladimir Sementsov-Ogievskiy (14):
python/qemu/qmp/legacy: cmd(): drop cmd_id unused argument
qmp_shell.py:
On Wed, Oct 04, 2023 at 04:55:02PM -0500, Eric Blake wrote:
> > > +static int
> > > +nbd_co_block_status_payload_read(NBDClient *client, NBDRequest *request,
> > > + Error **errp)
> >
> > [..]
> > > +for (i = 0; i < count; i++) {
> > > +id =
On Thu, 5 Oct 2023 at 00:49, Philippe Mathieu-Daudé wrote:
>
> Hi John,
>
> On 4/10/23 21:46, John Snow wrote:
> > The following changes since commit da1034094d375afe9e3d8ec8980550ea0f06f7e0:
> >
> >Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging
> > (2023-10-03
On Wed, Oct 04, 2023 at 02:00:19PM +0200, Philippe Mathieu-Daudé wrote:
> Fix:
>
> trace/control.c:288:34: error: declaration shadows a variable in the global
> scope [-Werror,-Wshadow]
> void trace_opt_parse(const char *optarg)
>^
>
>
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/8.2 for any
user-visible changes.
signature.asc
Description: PGP signature
On 26.07.23 16:32, Thomas Huth wrote:
On 26/07/2023 15.00, Peter Maydell wrote:
On Wed, 26 Jul 2023 at 13:06, Juan Quintela wrote:
To make things easier, this is the part that show how it breaks (this is
the gcov test):
357/423 qemu:block / io-qcow2-copy-before-write
Please ignore this copy, it has the recipients messed up.
Philippe Mathieu-Daudé writes:
> Address the comment added in commit 4629ed1e98
> ("qerror: Finally unused, clean up"), from 2015:
>
> /*
>* These macros will go away, please don't use
>* in new code, and do not add new ones!
>*/
>
> Mechanical transformation using:
>
> $ sed -i
Philippe Mathieu-Daudé writes:
> Address the comment added in commit 4629ed1e98
> ("qerror: Finally unused, clean up"), from 2015:
>
> /*
>* These macros will go away, please don't use
>* in new code, and do not add new ones!
>*/
>
> Mechanical transformation using:
>
> $ sed -i
On Wednesday, 6 September 2023 Fiona Ebner wrote:
> If there is a pending DMA operation during ide_bus_reset(), the fact
> that the IDEState is already reset before the operation is canceled
> can be problematic. In particular, ide_dma_cb() might be called and
> then use the reset IDEState which
On 5/10/23 10:59, Markus Armbruster wrote:
Philippe Mathieu-Daudé writes:
Fix:
softmmu/vl.c:1069:44: error: declaration shadows a variable in the global
scope [-Werror,-Wshadow]
static void parse_display_qapi(const char *optarg)
^
From: Niklas Cassel
Legacy software contains a standard mechanism for generating a reset to a
Serial ATA device - setting the SRST (software reset) bit in the Device
Control register.
Serial ATA has a more robust mechanism called COMRESET, also referred to
as port reset. A port reset is the
From: Niklas Cassel
Legacy software contains a standard mechanism for generating a reset to a
Serial ATA device - setting the SRST (software reset) bit in the Device
Control register.
Serial ATA has a more robust mechanism called COMRESET, also referred to
as port reset. A port reset is the
Philippe Mathieu-Daudé writes:
> Since v1:
> - Fixed checkpatch warnings (Juan)
> - Added R-b tags
> - New patch for 'vcpu_dirty_limit'
>
> Hi,
>
> This is kind of a selfish series. I'm really tired to grep
> and read this comment from 2015 in qapi/qmp/qerror.h:
> /*
>* These
Philippe Mathieu-Daudé writes:
> Fix:
>
> softmmu/vl.c:1069:44: error: declaration shadows a variable in the global
> scope [-Werror,-Wshadow]
> static void parse_display_qapi(const char *optarg)
> ^
> softmmu/vl.c:1224:39: error: declaration
Am 04.10.23 um 18:13 schrieb Vladimir Sementsov-Ogievskiy:
>> @@ -833,7 +836,10 @@ static int dirty_bitmap_load_start(QEMUFile *f,
>> DBMLoadState *s)
>> bdrv_disable_dirty_bitmap(s->bitmap);
>> if (flags & DIRTY_BITMAP_MIG_START_FLAG_ENABLED) {
>> + AioContext *ctx =
Philippe Mathieu-Daudé wrote:
> vcpu_dirty_limit
>
> Address the comment added in commit 4629ed1e98
> ("qerror: Finally unused, clean up"), from 2015:
>
> /*
>* These macros will go away, please don't use
>* in new code, and do not add new ones!
>*/
>
> Mechanical transformation
Philippe Mathieu-Daudé wrote:
> QERR_INVALID_PARAMETER_VALUE is defined as:
>
> #define QERR_INVALID_PARAMETER_VALUE \
> "Parameter '%s' expects %s"
>
> The current error is formatted as:
>
> "Parameter 'vcpu_dirty_limit' expects is invalid, it must greater then 1
> MB/s"
>
> Replace
68 matches
Mail list logo
