Re: [PATCH v4 07/30] qcow2: Document the Extended L2 Entries feature

2020-04-15 Thread Eric Blake
On 4/15/20 2:11 PM, Alberto Garcia wrote: On Fri 10 Apr 2020 11:29:59 AM CEST, Vladimir Sementsov-Ogievskiy wrote: Should we also document that extended L2 entries are incompatible with raw external files? [...] After all, when raw external file is enabled, the entire image is allocated, at

Re: [PATCH for-5.1] qcow2: Don't open images with a backing file and the data-file-raw bit

2020-04-15 Thread Eric Blake
On 4/15/20 2:02 PM, Alberto Garcia wrote: Although we cannot create these images with qemu-img it is still possible to do it using an external tool. QEMU should refuse to open them until the data-file-raw bit is cleared with 'qemu-img check'. Signed-off-by: Alberto Garcia --- block/qcow2.c

Re: [PATCH v4 07/30] qcow2: Document the Extended L2 Entries feature

2020-04-15 Thread Alberto Garcia
On Fri 10 Apr 2020 11:29:59 AM CEST, Vladimir Sementsov-Ogievskiy wrote: >> Should we also document that extended L2 entries are incompatible >> with raw external files? [...] After all, when raw external file is >> enabled, the entire image is allocated, at which point subclusters >> don't make

[PATCH for-5.1] qcow2: Don't open images with a backing file and the data-file-raw bit

2020-04-15 Thread Alberto Garcia
Although we cannot create these images with qemu-img it is still possible to do it using an external tool. QEMU should refuse to open them until the data-file-raw bit is cleared with 'qemu-img check'. Signed-off-by: Alberto Garcia --- block/qcow2.c | 39

Re: [PATCH-for-5.0 04/12] qga: Restrict guest-file-read count to 48 MB to avoid crashes

2020-04-15 Thread Michael Roth
Quoting Philippe Mathieu-Daudé (2020-04-15 08:02:18) > On 4/15/20 2:34 PM, Daniel P. Berrangé wrote: > > On Tue, Apr 14, 2020 at 03:30:44PM +0200, Philippe Mathieu-Daudé wrote: > >> On [*] Daniel Berrangé commented: > >> > >>The QEMU guest agent protocol is not sensible way to access huge > >>

Re: [PATCH for-5.1 1/8] tests-qemu-opts: Cover has_help_option(), qemu_opt_has_help_opt()

2020-04-15 Thread Markus Armbruster
Kevin Wolf writes: > Am 14.04.2020 um 22:13 hat Markus Armbruster geschrieben: >> Kevin Wolf writes: >> >> > Am 14.04.2020 um 15:36 hat Markus Armbruster geschrieben: >> >> Kevin Wolf writes: >> >> > Am 14.04.2020 um 11:10 hat Markus Armbruster geschrieben: >> >> >> Eric Blake writes: >> >>

Re: [PATCH v2 00/16] nvme: refactoring and cleanups

2020-04-15 Thread no-reply
Patchew URL: https://patchew.org/QEMU/20200415130159.611361-1-...@irrelevant.dk/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [PATCH v2 00/16] nvme: refactoring and cleanups Message-id: 20200415130159.611361-1-...@irrelevant.dk

Re: [PATCH 00/16] nvme: refactoring and cleanups

2020-04-15 Thread no-reply
Patchew URL: https://patchew.org/QEMU/20200415102445.564803-1-...@irrelevant.dk/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [PATCH 00/16] nvme: refactoring and cleanups Message-id: 20200415102445.564803-1-...@irrelevant.dk Type:

Re: [PATCH v2 13/16] nvme: factor out namespace setup

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 3:20 PM, Klaus Birkelund Jensen wrote: On Apr 15 15:16, Philippe Mathieu-Daudé wrote: On 4/15/20 3:01 PM, Klaus Jensen wrote: From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 46 ++ 1 file changed, 26

Re: [PATCH v2 13/16] nvme: factor out namespace setup

2020-04-15 Thread Klaus Birkelund Jensen
On Apr 15 15:16, Philippe Mathieu-Daudé wrote: > On 4/15/20 3:01 PM, Klaus Jensen wrote: > > From: Klaus Jensen > > > > Signed-off-by: Klaus Jensen > > --- > > hw/block/nvme.c | 46 ++ > > 1 file changed, 26 insertions(+), 20 deletions(-) > > > >

Re: [PATCH v2 13/16] nvme: factor out namespace setup

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 3:01 PM, Klaus Jensen wrote: From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 46 ++ 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index d5244102252c..2b007115c302

[PATCH v2 14/16] nvme: factor out pci setup

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 30 ++ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 2b007115c302..906ae595025a 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@

Re: [PATCH v2 14/16] nvme: factor out pci setup

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 3:01 PM, Klaus Jensen wrote: From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 30 ++ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 2b007115c302..906ae595025a 100644 ---

Re: [PATCH v2 01/16] nvme: fix pci doorbell size calculation

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 3:01 PM, Klaus Jensen wrote: From: Klaus Jensen The size of the BAR is 0x1000 (main registers) + 8 bytes for each queue. Currently, the size of the BAR is calculated like so: n->reg_size = pow2ceil(0x1004 + 2 * (n->num_queues + 1) * 4); Since the 'num_queues' parameter

[PATCH v2 16/16] nvme: factor out controller identify setup

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Signed-off-by: Klaus Jensen Reviewed-by: Philippe Mathieu-Daudé --- hw/block/nvme.c | 52 +++-- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 4c28d75e0fc8..804f24719dce

Re: [PATCH v2 09/16] nvme: factor out property/constraint checks

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 3:01 PM, Klaus Jensen wrote: From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 43 --- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 44856e873fd1..5f9ebbd6a1d5

Re: [PATCH v2 02/16] nvme: rename trace events to pci_nvme

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 3:01 PM, Klaus Jensen wrote: From: Klaus Jensen Change the prefix of all nvme device related trace events to 'pci_nvme' to not clash with trace events from the nvme block driver. Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 190

[PATCH v2 13/16] nvme: factor out namespace setup

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 46 ++ 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index d5244102252c..2b007115c302 100644 --- a/hw/block/nvme.c +++

Re: [PATCH v2 12/16] nvme: add namespace helpers

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 3:01 PM, Klaus Jensen wrote: From: Klaus Jensen Introduce some small helpers to make the next patches easier on the eye. Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 3 +-- hw/block/nvme.h | 16 2 files changed, 17 insertions(+), 2 deletions(-) diff

[PATCH v2 12/16] nvme: add namespace helpers

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Introduce some small helpers to make the next patches easier on the eye. Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 3 +-- hw/block/nvme.h | 16 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index

Re: [PATCH v2 11/16] nvme: factor out block backend setup

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 3:01 PM, Klaus Jensen wrote: From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 45a352b63d89..80da0825d295 100644 --- a/hw/block/nvme.c +++

[PATCH v2 15/16] nvme: factor out cmb setup

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Signed-off-by: Klaus Jensen Reviewed-by: Philippe Mathieu-Daudé --- hw/block/nvme.c | 49 +++-- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 906ae595025a..4c28d75e0fc8

Re: [PATCH v2 16/16] nvme: factor out controller identify setup

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 3:01 PM, Klaus Jensen wrote: From: Klaus Jensen Signed-off-by: Klaus Jensen Reviewed-by: Philippe Mathieu-Daudé --- hw/block/nvme.c | 52 +++-- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/hw/block/nvme.c

[PATCH v2 11/16] nvme: factor out block backend setup

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 45a352b63d89..80da0825d295 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -1351,6 +1351,13 @@

Re: [PATCH-for-5.0 04/12] qga: Restrict guest-file-read count to 48 MB to avoid crashes

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 2:34 PM, Daniel P. Berrangé wrote: On Tue, Apr 14, 2020 at 03:30:44PM +0200, Philippe Mathieu-Daudé wrote: On [*] Daniel Berrangé commented: The QEMU guest agent protocol is not sensible way to access huge files inside the guest. It requires the inefficient process of

[PATCH v2 01/16] nvme: fix pci doorbell size calculation

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen The size of the BAR is 0x1000 (main registers) + 8 bytes for each queue. Currently, the size of the BAR is calculated like so: n->reg_size = pow2ceil(0x1004 + 2 * (n->num_queues + 1) * 4); Since the 'num_queues' parameter already accounts for the admin queue, this should

[PATCH v2 07/16] nvme: add max_ioqpairs device parameter

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen The num_queues device paramater has a slightly confusing meaning because it accounts for the admin queue pair which is not really optional. Secondly, it is really a maximum value of queues allowed. Add a new max_ioqpairs parameter that only accounts for I/O queue pairs, but

[PATCH v2 02/16] nvme: rename trace events to pci_nvme

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Change the prefix of all nvme device related trace events to 'pci_nvme' to not clash with trace events from the nvme block driver. Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 190 +- hw/block/trace-events | 172

[PATCH v2 05/16] nvme: use constants in identify

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Signed-off-by: Klaus Jensen Reviewed-by: Maxim Levitsky Reviewed-by: Philippe Mathieu-Daudé --- hw/block/nvme.c | 8 include/block/nvme.h | 8 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index

[PATCH v2 09/16] nvme: factor out property/constraint checks

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 43 --- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 44856e873fd1..5f9ebbd6a1d5 100644 --- a/hw/block/nvme.c +++

[PATCH v2 04/16] nvme: move device parameters to separate struct

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Move device configuration parameters to separate struct to make it explicit what is configurable and what is set internally. Signed-off-by: Klaus Jensen Signed-off-by: Klaus Jensen Acked-by: Keith Busch Reviewed-by: Maxim Levitsky Reviewed-by: Philippe Mathieu-Daudé ---

[PATCH v2 08/16] nvme: remove redundant cmbloc/cmbsz members

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Signed-off-by: Klaus Jensen Reviewed-by: Philippe Mathieu-Daudé --- hw/block/nvme.c | 7 ++- hw/block/nvme.h | 2 -- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 8092c1b46eb1..44856e873fd1 100644 ---

[PATCH v2 06/16] nvme: refactor nvme_addr_read

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Pull the controller memory buffer check to its own function. The check will be used on its own in later patches. Signed-off-by: Klaus Jensen Acked-by: Keith Busch Reviewed-by: Philippe Mathieu-Daudé --- hw/block/nvme.c | 16 1 file changed, 12

[PATCH v2 03/16] nvme: remove superfluous breaks

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen These break statements was left over when commit 3036a626e9ef ("nvme: add Get/Set Feature Timestamp support") was merged. Signed-off-by: Klaus Jensen Acked-by: Keith Busch Reviewed-by: Maxim Levitsky Reviewed-by: Philippe Mathieu-Daudé --- hw/block/nvme.c | 4 1

[PATCH v2 00/16] nvme: refactoring and cleanups

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Changes since v1 * nvme: fix pci doorbell size calculation - added some defines and a better comment (Philippe) * nvme: rename trace events to pci_nvme - changed the prefix from nvme_dev to pci_nvme (Philippe) * nvme: add max_ioqpairs device parameter

[PATCH v2 10/16] nvme: factor out device state setup

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Signed-off-by: Klaus Jensen Reviewed-by: Philippe Mathieu-Daudé --- hw/block/nvme.c | 22 +- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 5f9ebbd6a1d5..45a352b63d89 100644 --- a/hw/block/nvme.c

Re: [PATCH v2 for-5.1 9/9] qemu-img: Reject broken -o ""

2020-04-15 Thread Eric Blake
On 4/15/20 2:49 AM, Markus Armbruster wrote: qemu-img create, convert, amend, and measure use accumulate_options() to merge multiple -o options. This is broken for -o "": $ qemu-img create -f qcow2 -o backing_file=a -o "" -o backing_fmt=raw,size=1M new.qcow2 qemu-img: warning: Could

Re: [PATCH v2 for-5.1 4/9] qemu-option: Fix has_help_option()'s sloppy parsing

2020-04-15 Thread Eric Blake
On 4/15/20 2:49 AM, Markus Armbruster wrote: has_help_option() uses its own parser. It's inconsistent with qemu_opts_parse(), as demonstrated by test-qemu-opts case /qemu-opts/has_help_option. Fix by reusing the common parser. Signed-off-by: Markus Armbruster --- tests/test-qemu-opts.c |

Re: [PATCH v2 for-5.1 1/9] tests-qemu-opts: Cover has_help_option(), qemu_opt_has_help_opt()

2020-04-15 Thread Eric Blake
On 4/15/20 2:49 AM, Markus Armbruster wrote: The two turn out to be inconsistent for "a,b,,help". Test case marked /* BUG */. Signed-off-by: Markus Armbruster --- tests/test-qemu-opts.c | 44 ++ 1 file changed, 44 insertions(+) Reviewed-by: Eric

Re: [PATCH-for-5.0 04/12] qga: Restrict guest-file-read count to 48 MB to avoid crashes

2020-04-15 Thread Daniel P . Berrangé
On Tue, Apr 14, 2020 at 03:30:44PM +0200, Philippe Mathieu-Daudé wrote: > On [*] Daniel Berrangé commented: > > The QEMU guest agent protocol is not sensible way to access huge > files inside the guest. It requires the inefficient process of > reading the entire data into memory than

Re: [PATCH 02/16] nvme: rename trace events to nvme_dev

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 12:24 PM, Klaus Jensen wrote: From: Klaus Jensen Change the prefix of all nvme device related trace events to 'nvme_dev' Another option is 'pci_nvme'. None of them make me happy but I can't find a better name. to not clash with trace events from the nvme block driver.

Re: [PATCH 01/16] nvme: fix pci doorbell size calculation

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 12:24 PM, Klaus Jensen wrote: From: Klaus Jensen The size of the BAR is 0x1000 (main registers) + 8 bytes for each queue. Currently, the size of the BAR is calculated like so: n->reg_size = pow2ceil(0x1004 + 2 * (n->params.num_queues + 1) * 4); Since the 'num_queues'

Re: [PATCH 11/16] nvme: factor out block backend setup

2020-04-15 Thread Klaus Birkelund Jensen
On Apr 15 13:02, Klaus Birkelund Jensen wrote: > On Apr 15 12:52, Philippe Mathieu-Daudé wrote: > > On 4/15/20 12:24 PM, Klaus Jensen wrote: > > > From: Klaus Jensen > > > > > > Signed-off-by: Klaus Jensen > > > --- > > > hw/block/nvme.c | 15 --- > > > 1 file changed, 12

Re: [PATCH 16/16] nvme: factor out controller identify setup

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 12:24 PM, Klaus Jensen wrote: From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 42 -- 1 file changed, 24 insertions(+), 18 deletions(-) The review is trivial using 'git-diff -W'. Reviewed-by: Philippe Mathieu-Daudé

Re: [PATCH 11/16] nvme: factor out block backend setup

2020-04-15 Thread Klaus Birkelund Jensen
On Apr 15 12:52, Philippe Mathieu-Daudé wrote: > On 4/15/20 12:24 PM, Klaus Jensen wrote: > > From: Klaus Jensen > > > > Signed-off-by: Klaus Jensen > > --- > > hw/block/nvme.c | 15 --- > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/hw/block/nvme.c

Re: [PATCH 07/16] nvme: add max_ioqpairs device parameter

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 12:24 PM, Klaus Jensen wrote: From: Klaus Jensen The num_queues device paramater has a slightly confusing meaning because it accounts for the admin queue pair which is not really optional. Secondly, it is really a maximum value of queues allowed. Add a new max_ioqpairs parameter

Re: [PATCH 13/16] nvme: factor out namespace setup

2020-04-15 Thread Klaus Birkelund Jensen
On Apr 15 12:53, Klaus Birkelund Jensen wrote: > On Apr 15 12:38, Philippe Mathieu-Daudé wrote: > > > > I'm not sure this line belong to this patch. > > > > It does. It is already there in the middle of the realize function. It > is moved to nvme_init_namespace as > > n->ns_size = bs_size;

Re: [PATCH 09/16] nvme: factor out property/constraint checks

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 12:24 PM, Klaus Jensen wrote: From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 52 ++--- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index

Re: [PATCH 13/16] nvme: factor out namespace setup

2020-04-15 Thread Klaus Birkelund Jensen
On Apr 15 12:38, Philippe Mathieu-Daudé wrote: > On 4/15/20 12:24 PM, Klaus Jensen wrote: > > From: Klaus Jensen > > > > Signed-off-by: Klaus Jensen > > --- > > hw/block/nvme.c | 47 ++- > > 1 file changed, 26 insertions(+), 21 deletions(-) > > >

Re: [PATCH 10/16] nvme: factor out device state setup

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 12:24 PM, Klaus Jensen wrote: From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 635292d6fac4..e67f578fbf79 100644 --- a/hw/block/nvme.c

Re: [PATCH 11/16] nvme: factor out block backend setup

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 12:24 PM, Klaus Jensen wrote: From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index e67f578fbf79..f0989cbb4335 100644 --- a/hw/block/nvme.c

Re: [PATCH 15/16] nvme: factor out cmb setup

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 12:24 PM, Klaus Jensen wrote: From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 50 +++-- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index

Re: [PATCH 14/16] nvme: factor out pci setup

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 12:24 PM, Klaus Jensen wrote: From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 32 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 08f7ae0a48b3..16d01af53a07 100644 ---

Re: [PATCH 12/16] nvme: add namespace helpers

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 12:24 PM, Klaus Jensen wrote: From: Klaus Jensen Introduce some small helpers to make the next patches easier on the eye. Signed-off-by: Klaus Jensen --- hw/block/nvme.h | 16 1 file changed, 16 insertions(+) diff --git a/hw/block/nvme.h b/hw/block/nvme.h

Re: [PATCH 13/16] nvme: factor out namespace setup

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 12:24 PM, Klaus Jensen wrote: From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 47 ++- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index

[PATCH 02/16] nvme: rename trace events to nvme_dev

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Change the prefix of all nvme device related trace events to 'nvme_dev' to not clash with trace events from the nvme block driver. Signed-off-by: Klaus Jensen Acked-by: Keith Busch Reviewed-by: Maxim Levitsky --- hw/block/nvme.c | 190

[PATCH 11/16] nvme: factor out block backend setup

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index e67f578fbf79..f0989cbb4335 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -1348,6 +1348,17 @@

[PATCH 12/16] nvme: add namespace helpers

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Introduce some small helpers to make the next patches easier on the eye. Signed-off-by: Klaus Jensen --- hw/block/nvme.h | 16 1 file changed, 16 insertions(+) diff --git a/hw/block/nvme.h b/hw/block/nvme.h index ad1786953be9..d9900bed957c 100644 ---

[PATCH 06/16] nvme: refactor nvme_addr_read

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Pull the controller memory buffer check to its own function. The check will be used on its own in later patches. Signed-off-by: Klaus Jensen Acked-by: Keith Busch Reviewed-by: Philippe Mathieu-Daudé --- hw/block/nvme.c | 16 1 file changed, 12

[PATCH 13/16] nvme: factor out namespace setup

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 47 ++- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index f0989cbb4335..08f7ae0a48b3 100644 --- a/hw/block/nvme.c +++

[PATCH 14/16] nvme: factor out pci setup

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 32 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 08f7ae0a48b3..16d01af53a07 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@

[PATCH 05/16] nvme: use constants in identify

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Signed-off-by: Klaus Jensen Reviewed-by: Maxim Levitsky Reviewed-by: Philippe Mathieu-Daudé --- hw/block/nvme.c | 8 include/block/nvme.h | 8 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index

[PATCH 16/16] nvme: factor out controller identify setup

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 42 -- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 7387cf409f96..d1566b56381d 100644 --- a/hw/block/nvme.c +++

[PATCH 15/16] nvme: factor out cmb setup

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 50 +++-- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 16d01af53a07..7387cf409f96 100644 --- a/hw/block/nvme.c +++

[PATCH 08/16] nvme: remove redundant cmbloc/cmbsz members

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Signed-off-by: Klaus Jensen Reviewed-by: Philippe Mathieu-Daudé --- hw/block/nvme.c | 7 ++- hw/block/nvme.h | 2 -- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 9383d2cb0b38..ea613213bd57 100644 ---

[PATCH 04/16] nvme: move device parameters to separate struct

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Move device configuration parameters to separate struct to make it explicit what is configurable and what is set internally. Signed-off-by: Klaus Jensen Signed-off-by: Klaus Jensen Acked-by: Keith Busch Reviewed-by: Maxim Levitsky Reviewed-by: Philippe Mathieu-Daudé ---

[PATCH 07/16] nvme: add max_ioqpairs device parameter

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen The num_queues device paramater has a slightly confusing meaning because it accounts for the admin queue pair which is not really optional. Secondly, it is really a maximum value of queues allowed. Add a new max_ioqpairs parameter that only accounts for I/O queue pairs, but

[PATCH 10/16] nvme: factor out device state setup

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 635292d6fac4..e67f578fbf79 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -1339,6 +1339,15 @@

[PATCH 09/16] nvme: factor out property/constraint checks

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 52 ++--- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index ea613213bd57..635292d6fac4 100644 --- a/hw/block/nvme.c +++

[PATCH 01/16] nvme: fix pci doorbell size calculation

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen The size of the BAR is 0x1000 (main registers) + 8 bytes for each queue. Currently, the size of the BAR is calculated like so: n->reg_size = pow2ceil(0x1004 + 2 * (n->params.num_queues + 1) * 4); Since the 'num_queues' parameter already accounts for the admin queue, this

[PATCH 03/16] nvme: remove superfluous breaks

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen These break statements was left over when commit 3036a626e9ef ("nvme: add Get/Set Feature Timestamp support") was merged. Signed-off-by: Klaus Jensen Acked-by: Keith Busch Reviewed-by: Maxim Levitsky Reviewed-by: Philippe Mathieu-Daudé --- hw/block/nvme.c | 4 1

[PATCH 00/16] nvme: refactoring and cleanups

2020-04-15 Thread Klaus Jensen
From: Klaus Jensen Philippe suggested that I split up this already way too huge series (more than 50 patches now), so here goes. The first patch in this series fixes a small bug in the pci doorbell size calculation. Please consider cherry-picking this. The rest are refactorings. The "nvme: add

Re: [PATCH for-5.1 1/8] tests-qemu-opts: Cover has_help_option(), qemu_opt_has_help_opt()

2020-04-15 Thread Kevin Wolf
Am 14.04.2020 um 22:13 hat Markus Armbruster geschrieben: > Kevin Wolf writes: > > > Am 14.04.2020 um 15:36 hat Markus Armbruster geschrieben: > >> Kevin Wolf writes: > >> > Am 14.04.2020 um 11:10 hat Markus Armbruster geschrieben: > >> >> Eric Blake writes: > >> >> > On 4/9/20 10:30 AM,

Re: [PATCH v2 for-5.1 0/9] qemu-option: Fix corner cases and clean up

2020-04-15 Thread no-reply
Patchew URL: https://patchew.org/QEMU/20200415074927.19897-1-arm...@redhat.com/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [PATCH v2 for-5.1 0/9] qemu-option: Fix corner cases and clean up Message-id:

Re: [PATCH v4 17/30] qcow2: Add subcluster support to calculate_l2_meta()

2020-04-15 Thread Vladimir Sementsov-Ogievskiy
17.03.2020 21:16, Alberto Garcia wrote: If an image has subclusters then there are more copy-on-write scenarios that we need to consider. Let's say we have a write request from the middle of subcluster #3 until the end of the cluster: - If the cluster is new, then subclusters #0 to #3 from

Re: [PATCH v7 11/48] nvme: refactor device realization

2020-04-15 Thread Klaus Birkelund Jensen
On Apr 15 09:55, Philippe Mathieu-Daudé wrote: > On 4/15/20 9:25 AM, Klaus Birkelund Jensen wrote: > > On Apr 15 09:14, Philippe Mathieu-Daudé wrote: > > > Hi Klaus, > > > > > > This patch is a pain to review... Could you split it? I'd use one trivial > > > patch for each function extracted from

Re: [PATCH-for-5.0 10/12] hw/block/pflash: Check return value of blk_pwrite()

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/14/20 3:30 PM, Philippe Mathieu-Daudé wrote: From: Mansour Ahmadi When updating the PFLASH file contents, we should check for a possible failure of blk_pwrite(). Similar to commit 3a688294e. There is actually a Coverity report for this issue, CID 1357678 (Unchecked return value) from

Re: [PATCH v7 00/48] nvme: support NVMe v1.3d, SGLs and multiple namespaces

2020-04-15 Thread no-reply
Patchew URL: https://patchew.org/QEMU/20200415055140.466900-1-...@irrelevant.dk/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [PATCH v7 00/48] nvme: support NVMe v1.3d, SGLs and multiple namespaces Message-id:

Re: [PATCH v7 45/48] nvme: support multiple namespaces

2020-04-15 Thread Klaus Birkelund Jensen
On Apr 15 09:38, Philippe Mathieu-Daudé wrote: > On 4/15/20 7:51 AM, Klaus Jensen wrote: > > From: Klaus Jensen > > > > This adds support for multiple namespaces by introducing a new 'nvme-ns' > > device model. The nvme device creates a bus named from the device name > > ('id'). The nvme-ns

Re: [PATCH-for-5.1 v3 18/24] hw/pci-host/pnv_phb3: Move some code from realize() to init()

2020-04-15 Thread Cédric Le Goater
On 4/13/20 12:36 AM, Philippe Mathieu-Daudé wrote: > Coccinelle reported: > > $ spatch ... --timeout 60 --sp-file \ > scripts/coccinelle/simplify-init-realize-error_propagate.cocci > HANDLING: ./hw/pci-host/pnv_phb3.c > >>> possible moves from pnv_phb3_instance_init() to

[PATCH v2 for-5.1 2/9] qemu-options: Factor out get_opt_name_value() helper

2020-04-15 Thread Markus Armbruster
The next commits will put it to use. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf --- util/qemu-option.c | 102 + 1 file changed, 56 insertions(+), 46 deletions(-) diff --git a/util/qemu-option.c

Re: [PATCH v7 11/48] nvme: refactor device realization

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 9:25 AM, Klaus Birkelund Jensen wrote: On Apr 15 09:14, Philippe Mathieu-Daudé wrote: Hi Klaus, This patch is a pain to review... Could you split it? I'd use one trivial patch for each function extracted from nvme_realize(). Understood, I will split it up! Thanks, that will

Re: [PATCH] block/nvme: Remove memory leak

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 9:46 AM, Stefano Garzarella wrote: On Tue, Apr 14, 2020 at 05:17:26PM +0200, Philippe Mathieu-Daudé wrote: Fixes: bdd6a90a9 ("Add VFIO based NVMe driver") Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/nvme.c

[PATCH v2 for-5.1 9/9] qemu-img: Reject broken -o ""

2020-04-15 Thread Markus Armbruster
qemu-img create, convert, amend, and measure use accumulate_options() to merge multiple -o options. This is broken for -o "": $ qemu-img create -f qcow2 -o backing_file=a -o "" -o backing_fmt=raw,size=1M new.qcow2 qemu-img: warning: Could not verify backing image. This may become an

[PATCH v2 for-5.1 4/9] qemu-option: Fix has_help_option()'s sloppy parsing

2020-04-15 Thread Markus Armbruster
has_help_option() uses its own parser. It's inconsistent with qemu_opts_parse(), as demonstrated by test-qemu-opts case /qemu-opts/has_help_option. Fix by reusing the common parser. Signed-off-by: Markus Armbruster --- tests/test-qemu-opts.c | 4 ++-- util/qemu-option.c | 39

[PATCH v2 for-5.1 8/9] qemu-img: Move is_valid_option_list() to qemu-img.c and rewrite

2020-04-15 Thread Markus Armbruster
is_valid_option_list()'s purpose is ensuring qemu-img.c's can safely join multiple parameter strings separated by ',' like this: g_strdup_printf("%s,%s", params1, params2); How it does that is anything but obvious. A close reading of the code reveals that it fails exactly when its

[PATCH v2 for-5.1 5/9] test-qemu-opts: Simplify test_has_help_option() after bug fix

2020-04-15 Thread Markus Armbruster
Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf --- tests/test-qemu-opts.c | 36 +--- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c index 77c944c4aa..2a0f42a09b

Re: [PATCH-for-5.1 v3 05/24] hw/arm/aspeed_ast2600: Move some code from realize() to init()

2020-04-15 Thread Cédric Le Goater
On 4/13/20 12:36 AM, Philippe Mathieu-Daudé wrote: > Coccinelle reported: > > $ spatch ... --timeout 60 --sp-file \ > scripts/coccinelle/simplify-init-realize-error_propagate.cocci > HANDLING: ./hw/arm/aspeed_ast2600.c > >>> possible moves from aspeed_soc_ast2600_init() to >

[PATCH v2 for-5.1 1/9] tests-qemu-opts: Cover has_help_option(), qemu_opt_has_help_opt()

2020-04-15 Thread Markus Armbruster
The two turn out to be inconsistent for "a,b,,help". Test case marked /* BUG */. Signed-off-by: Markus Armbruster --- tests/test-qemu-opts.c | 44 ++ 1 file changed, 44 insertions(+) diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c index

[PATCH v2 for-5.1 3/9] qemu-option: Fix sloppy recognition of "id=..." after ", , "

2020-04-15 Thread Markus Armbruster
Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf --- tests/test-qemu-opts.c | 4 ++-- util/qemu-option.c | 27 +++ 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c index

[PATCH v2 for-5.1 6/9] qemu-option: Avoid has_help_option() in qemu_opts_parse_noisily()

2020-04-15 Thread Markus Armbruster
When opts_parse() sets @invalidp to true, qemu_opts_parse_noisily() uses has_help_option() to decide whether to print help. This parses the input string a second time. Easy to avoid: replace @invalidp by @help_wanted. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake ---

[PATCH v2 for-5.1 7/9] qemu-img: Factor out accumulate_options() helper

2020-04-15 Thread Markus Armbruster
Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf --- qemu-img.c | 59 +- 1 file changed, 23 insertions(+), 36 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index 821cbf610e..d36b21b758 100644 ---

[PATCH v2 for-5.1 0/9] qemu-option: Fix corner cases and clean up

2020-04-15 Thread Markus Armbruster
v2: * PATCH 1,4,5: Cover "?" in addition to "help" [Kevin] * PATCH 4-6: Old PATCH 4 moved after old PATCH 5+6, commit message adjusted * PATCH 4: Unbreak "?" [Eric] * PATCH 8: Commit message tweaked * PATCH 9: New Markus Armbruster (9): tests-qemu-opts: Cover has_help_option(),

Re: [PATCH-for-5.1 1/3] target: Remove unnecessary CPU() cast

2020-04-15 Thread Cédric Le Goater
On 4/12/20 11:09 PM, Philippe Mathieu-Daudé wrote: > The CPU() macro is defined as: > > #define CPU(obj) ((CPUState *)(obj)) > > Remove an unnecessary CPU() cast. > > Patch created mechanically using spatch with this script: > > @@ > typedef CPUState; > CPUState *s; > @@ > -

Re: [PATCH for-5.1 5/8] qemu-option: Fix has_help_option()'s sloppy parsing

2020-04-15 Thread Markus Armbruster
Kevin Wolf writes: > Am 14.04.2020 um 12:16 hat Markus Armbruster geschrieben: >> Eric Blake writes: >> >> > On 4/9/20 10:30 AM, Markus Armbruster wrote: >> >> has_help_option() uses its own parser. It's inconsistent with >> >> qemu_opts_parse(), as demonstrated by test-qemu-opts case >> >>

Re: [PATCH-for-5.1 3/3] hw: Remove unnecessary DEVICE() cast

2020-04-15 Thread Cédric Le Goater
On 4/12/20 11:09 PM, Philippe Mathieu-Daudé wrote: > The DEVICE() macro is defined as: > > #define DEVICE(obj) OBJECT_CHECK(DeviceState, (obj), TYPE_DEVICE) > > Remove unnecessary DEVICE() casts. > > Patch created mechanically using spatch with this script: > > @@ > typedef DeviceState;

Re: [PATCH v7 10/48] nvme: remove redundant cmbloc/cmbsz members

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 9:19 AM, Klaus Birkelund Jensen wrote: On Apr 15 09:10, Philippe Mathieu-Daudé wrote: "hw/block/nvme.h" should not pull in "block/nvme.h", both should include a common "hw/block/nvme_spec.h" (or better named). Not related to this patch although. Hmm. It does pull in the

Re: [PATCH] block/nvme: Remove memory leak

2020-04-15 Thread Stefano Garzarella
On Tue, Apr 14, 2020 at 05:17:26PM +0200, Philippe Mathieu-Daudé wrote: > Fixes: bdd6a90a9 ("Add VFIO based NVMe driver") > Signed-off-by: Philippe Mathieu-Daudé > --- > block/nvme.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/block/nvme.c b/block/nvme.c > index

Re: [PATCH v7 06/48] nvme: refactor nvme_addr_read

2020-04-15 Thread Klaus Birkelund Jensen
On Apr 15 09:03, Philippe Mathieu-Daudé wrote: > On 4/15/20 7:50 AM, Klaus Jensen wrote: > > From: Klaus Jensen > > > > Pull the controller memory buffer check to its own function. The check > > will be used on its own in later patches. > > > > Signed-off-by: Klaus Jensen > > Acked-by: Keith

Re: [PATCH v7 12/48] nvme: add temperature threshold feature

2020-04-15 Thread Philippe Mathieu-Daudé
On 4/15/20 9:28 AM, Klaus Birkelund Jensen wrote: On Apr 15 09:24, Klaus Birkelund Jensen wrote: On Apr 15 09:19, Philippe Mathieu-Daudé wrote: On 4/15/20 7:51 AM, Klaus Jensen wrote: From: Klaus Jensen It might seem wierd to implement this feature for an emulated device, 'weird'

  1   2   >