Re: [PULL 2/3] rbd: Fix .bdrv_get_specific_info implementation

[Kevin Wolf <kwolf@redhat.com>]

Am 13.08.2025 um 08:12 hat Michael Tokarev geschrieben:
> On 12.08.2025 22:59, Kevin Wolf wrote:
> > qemu_rbd_get_specific_info() has at least two problems:
> > 
> > The first is that it issues a blocking rbd_read() call in order to probe
> > the encryption format for the image while querying the node. This means
> > that if the connection to the server goes down, not only I/O is stuck
> > (which is unavoidable), but query-names-block-nodes will actually make
> > the whole QEMU instance unresponsive. .bdrv_get_specific_info
> > implementations shouldn't perform blocking operations, but only return
> > what is already known.
> > 
> > The second is that the information returned isn't even correct. If the
> > image is already opened with encryption enabled at the RBD level, we'll
> > probe for "double encryption", i.e. if the encrypted data contains
> > another encryption header. If it doesn't (which is the normal case), we
> > won't return the encryption format. If it does, we return misleading
> > information because it looks like we're talking about the outer level
> > (the encryption format of the image itself) while the information is
> > about an encryption header in the guest data.
> > 
> > Fix this by storing the encryption format in BDRVRBDState when the image
> > is opened (and we do blocking operations anyway) and returning only the
> > stored information in qemu_rbd_get_specific_info().
> > 
> > The information we'll store is either the actual encryption format that
> > we enabled on the RBD level, or if the image is unencrypted, the result
> > of the same probing as we previously did when querying the node. Probing
> > image formats based on content that can be modified by the guest has
> > long been known as problematic, but as long as we only output it to the
> > user instead of making decisions based on it, it should be okay. It is
> > undoubtedly useful in the context of 'qemu-img info' when you're trying
> > to figure out which encryption options you have to use to open the
> > image successfully.
> > 
> > Fixes: 42e4ac9ef5a6 ("block/rbd: Add support for rbd image encryption")
> > Buglink: https://issues.redhat.com/browse/RHEL-105440
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > Message-ID: <20250811134010.81787-1-kwolf@redhat.com>
> > Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> 
> This looks like a qemu-stable material (10.0).  Please let me know
> if it is not.

I was on the verge of adding Cc: qemu-stable for it, but then decided
against it because it does subtly change behaviour in the common case
where the connection doesn't go down. On second thoughts, the common use
case is 'qemu-img info' where the image isn't opened encrypted (because
we're only trying to find out which encryption format it has), and that
output stays the same. So taking it into stable might still be
reasonable enough.

Kevin