Re: [Libguestfs] [PATCH v4 09/24] nbd: Replace bool structured_reply with mode enum

2023-07-19 Thread Eric Blake
On Mon, Jun 12, 2023 at 02:24:52PM -0500, Eric Blake wrote: > On Mon, Jun 12, 2023 at 06:07:59PM +0300, Vladimir Sementsov-Ogievskiy wrote: > > On 08.06.23 16:56, Eric Blake wrote: > > > The upcoming patches for 64-bit extensions requires various points in > > > the protocol to make decisions

[PULL 01/14] qemu-nbd: pass structure into nbd_client_thread instead of plain char*

2023-07-19 Thread Eric Blake
From: "Denis V. Lunev" We are going to pass additional flag inside next patch. Signed-off-by: Denis V. Lunev CC: Eric Blake CC: Vladimir Sementsov-Ogievskiy CC: Message-ID: <20230717145544.194786-2-...@openvz.org> Reviewed-by: Eric Blake Signed-off-by: Eric Blake --- qemu-nbd.c | 19

[PULL 12/14] nbd/client: Simplify cookie vs. index computation

2023-07-19 Thread Eric Blake
Our code relies on a sentinel cookie value of zero for deciding when a packet has been handled, as well as relying on array indices between 0 and MAX_NBD_REQUESTS-1 for dereferencing purposes. As long as we can symmetrically convert between two forms, there is no reason to go with the odd choice

[PULL 04/14] qemu-nbd: properly report error on error in dup2() after qemu_daemon()

2023-07-19 Thread Eric Blake
From: "Denis V. Lunev" We are trying to temporarily redirect stderr of daemonized process to a pipe to report a error and get failed. In that case we could not use error_report() helper, but should write the message directly into the problematic pipe. Signed-off-by: Denis V. Lunev CC: Eric

[PULL 13/14] nbd/client: Add safety check on chunk payload length

2023-07-19 Thread Eric Blake
Our existing use of structured replies either reads into a qiov capped at 32M (NBD_CMD_READ) or caps allocation to 1000 bytes (see NBD_MAX_MALLOC_PAYLOAD in block/nbd.c). But the existing length checks are rather late; if we encounter a buggy (or malicious) server that sends a super-large payload

Re: [PULL 1/1] hw/nvme: fix endianness issue for shadow doorbells

2023-07-19 Thread Michael Tokarev
19.07.2023 10:36, Klaus Jensen wrote: pu(req->cmd.dptr.prp2); +uint32_t v; if (sq) { +v = cpu_to_le32(sq->tail); -pci_dma_write(pci, sq->db_addr, >tail, sizeof(sq->tail)); +pci_dma_write(pci, sq->db_addr, , sizeof(sq->tail)); This and

[PULL 08/14] nbd: Consistent typedef usage in header

2023-07-19 Thread Eric Blake
We had a mix of struct declarations followed by typedefs, and direct struct definitions as part of a typedef. Pick a single style. Also float forward declarations of opaque types to the top of the file, rather than interspersed with function declarations, which will help a future patch that

[PULL 11/14] nbd: s/handle/cookie/ to match NBD spec

2023-07-19 Thread Eric Blake
Externally, libnbd exposed the 64-bit opaque marker for each client NBD packet as the "cookie", because it was less confusing when contrasted with 'struct nbd_handle *' holding all libnbd state. It also avoids confusion between the noun 'handle' as a way to identify a packet and the verb 'handle'

[PULL 03/14] qemu-nbd: properly report error if qemu_daemon() is failed

2023-07-19 Thread Eric Blake
From: "Denis V. Lunev" errno has been overwritten by dup2() just below qemu_daemon() and thus improperly returned to the caller. Fix accordingly. Signed-off-by: Denis V. Lunev CC: Eric Blake CC: Vladimir Sementsov-Ogievskiy Message-ID: <20230717145544.194786-5-...@openvz.org> Reviewed-by:

[PULL 10/14] nbd/server: Refactor to pass full request around

2023-07-19 Thread Eric Blake
Part of NBD's 64-bit headers extension involves passing the client's requested offset back as part of the reply header (one reason it stated for this change: converting absolute offsets stored in NBD_REPLY_TYPE_OFFSET_DATA to relative offsets within the buffer is easier if the absolute offset of

[PULL 14/14] nbd: Use enum for various negotiation modes

2023-07-19 Thread Eric Blake
Deciphering the hard-coded list of integer return values from nbd_start_negotiate() will only get more confusing when adding support for 64-bit extended headers. Better is to name things in an enum. Although the function in question is private to client.c, putting the enum in a public header and

[PULL 05/14] qemu-nbd: handle dup2() error when qemu-nbd finished setup process

2023-07-19 Thread Eric Blake
From: "Denis V. Lunev" Fail on error, we are in trouble. Signed-off-by: Denis V. Lunev CC: Eric Blake CC: Vladimir Sementsov-Ogievskiy Message-ID: <20230717145544.194786-6-...@openvz.org> Reviewed-by: Eric Blake [eblake: avoid intermediate variable] Signed-off-by: Eric Blake ---

[PULL 02/14] qemu-nbd: fix regression with qemu-nbd --fork run over ssh

2023-07-19 Thread Eric Blake
From: "Denis V. Lunev" Commit e6df58a5578fee7a50bbf36f4a50a2781cff855d Author: Hanna Reitz Date: Wed May 8 23:18:18 2019 +0200 qemu-nbd: Do not close stderr has introduced an interesting regression. Original behavior of ssh somehost qemu-nbd /home/den/tmp/file -f raw --fork

[PULL 09/14] nbd/server: Prepare for alternate-size headers

2023-07-19 Thread Eric Blake
Upstream NBD now documents[1] an extension that supports 64-bit effect lengths in requests. As part of that extension, the size of the reply headers will change in order to permit a 64-bit length in the reply for symmetry[2]. Additionally, where the reply header is currently 16 bytes for simple

[PULL 07/14] nbd/client: Use smarter assert

2023-07-19 Thread Eric Blake
Assigning strlen() to a uint32_t and then asserting that it isn't too large doesn't catch the case of an input string 4G in length. Thankfully, the incoming strings can never be that large: if the export name or query is reflecting a string the client got from the server, we already guarantee that

[PULL 06/14] qemu-nbd: make verbose bool and local variable in main()

2023-07-19 Thread Eric Blake
From: "Denis V. Lunev" Pass 'verbose' to nbd_client_thread() inside NbdClientOpts which looks a little bit cleaner and make it bool as it is used as bool actually. Signed-off-by: Denis V. Lunev CC: Eric Blake CC: Vladimir Sementsov-Ogievskiy Message-ID:

Re: [PULL 0/1] hw/nvme fixes

2023-07-19 Thread Peter Maydell
On Wed, 19 Jul 2023 at 08:36, Klaus Jensen wrote: > > From: Klaus Jensen > > Hi, > > The following changes since commit 361d5397355276e3007825cc17217c1e4d4320f7: > > Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into > staging (2023-07-17 15:49:27 +0100) > > are available

[PATCH for-8.1] hw/nvme: fix compliance issue wrt. iosqes/iocqes

2023-07-19 Thread Klaus Jensen
From: Klaus Jensen As of prior to this patch, the controller checks the value of CC.IOCQES and CC.IOSQES prior to enabling the controller. As reported by Ben in GitLab issue #1691, this is not spec compliant. The controller should only check these values when queues are created. This patch

Re: [PATCH v3 8/8] hw/ide/ahci: fix broken SError handling

2023-07-19 Thread Philippe Mathieu-Daudé
On 19/7/23 14:06, Michael Tokarev wrote: 19.07.2023 14:59, Philippe Mathieu-Daudé wrote: On 9/6/23 16:08, Niklas Cassel wrote: From: Niklas Cassel When encountering an NCQ error, you should not write the NCQ tag to the SError register. This is completely wrong. The SError register has a

Re: [PATCH v3 8/8] hw/ide/ahci: fix broken SError handling

2023-07-19 Thread Michael Tokarev
19.07.2023 14:59, Philippe Mathieu-Daudé wrote: On 9/6/23 16:08, Niklas Cassel wrote: From: Niklas Cassel When encountering an NCQ error, you should not write the NCQ tag to the SError register. This is completely wrong. The SError register has a clear definition, where each bit represents a

Re: [PATCH v3 8/8] hw/ide/ahci: fix broken SError handling

2023-07-19 Thread Philippe Mathieu-Daudé
On 9/6/23 16:08, Niklas Cassel wrote: From: Niklas Cassel When encountering an NCQ error, you should not write the NCQ tag to the SError register. This is completely wrong. The SError register has a clear definition, where each bit represents a different error, see PxSERR definition in AHCI

Re: [PATCH v3 7/8] hw/ide/ahci: fix ahci_write_fis_sdb()

2023-07-19 Thread Philippe Mathieu-Daudé
On 9/6/23 16:08, Niklas Cassel wrote: From: Niklas Cassel When there is an error, we need to raise a TFES error irq, see AHCI 1.3.1, 5.3.13.1 SDB:Entry. If ERR_STAT is set, we jump to state ERR:FatalTaskfile, which will raise a TFES IRQ unconditionally, regardless if the I bit is set in the

Re: [PATCH v3 5/8] hw/ide/ahci: PxSACT and PxCI is cleared when PxCMD.ST is cleared

2023-07-19 Thread Philippe Mathieu-Daudé
On 9/6/23 16:08, Niklas Cassel wrote: From: Niklas Cassel According to AHCI 1.3.1 definition of PxSACT: This field is cleared when PxCMD.ST is written from a '1' to a '0' by software. This field is not cleared by a COMRESET or a software reset. Interesting, since its origin in commit

Re: [PATCH v3 2/8] hw/ide/core: set ERR_STAT in unsupported command completion

2023-07-19 Thread Philippe Mathieu-Daudé
On 9/6/23 16:08, Niklas Cassel wrote: From: Niklas Cassel Currently, the first time sending an unsupported command (e.g. READ LOG DMA EXT) will not have ERR_STAT set in the completion. Sending the unsupported command again, will correctly have ERR_STAT set. When ide_cmd_permitted() returns

Re: [PATCH v3 0/8] misc AHCI cleanups

2023-07-19 Thread Niklas Cassel
On Fri, Jun 09, 2023 at 04:08:36PM +0200, Niklas Cassel wrote: > From: Niklas Cassel > > Hello John, > > Here comes some misc AHCI cleanups. > > Most are related to error handling. > > Please review. > > Changes since v2: > -Squashed in the test commits that were sent out as a separate

[PULL 1/1] hw/nvme: fix endianness issue for shadow doorbells

2023-07-19 Thread Klaus Jensen
From: Klaus Jensen In commit 2fda0726e514 ("hw/nvme: fix missing endian conversions for doorbell buffers"), we fixed shadow doorbells for big-endian guests running on little endian hosts. But I did not fix little-endian guests on big-endian hosts. Fix this. Resolves:

[PULL 0/1] hw/nvme fixes

2023-07-19 Thread Klaus Jensen
From: Klaus Jensen Hi, The following changes since commit 361d5397355276e3007825cc17217c1e4d4320f7: Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into staging (2023-07-17 15:49:27 +0100) are available in the Git repository at: https://gitlab.com/birkelund/qemu.git