Skip to content
Snippets Groups Projects
  • Hanna Reitz's avatar
    998c2019
    block: Add BDS.auto_backing_file · 998c2019
    Hanna Reitz authored
    
    If the backing file is overridden, this most probably does change the
    guest-visible data of a BDS.  Therefore, we will need to consider this
    in bdrv_refresh_filename().
    
    To see whether it has been overridden, we might want to compare
    bs->backing_file and bs->backing->bs->filename.  However,
    bs->backing_file is changed by bdrv_set_backing_hd() (which is just used
    to change the backing child at runtime, without modifying the image
    header), so bs->backing_file most of the time simply contains a copy of
    bs->backing->bs->filename anyway, so it is useless for such a
    comparison.
    
    This patch adds an auto_backing_file BDS field which contains the
    backing file path as indicated by the image header, which is not changed
    by bdrv_set_backing_hd().
    
    Because of bdrv_refresh_filename() magic, however, a BDS's filename may
    differ from what has been specified during bdrv_open().  Then, the
    comparison between bs->auto_backing_file and bs->backing->bs->filename
    may fail even though bs->backing was opened from bs->auto_backing_file.
    To mitigate this, we can copy the real BDS's filename (after the whole
    bdrv_open() and bdrv_refresh_filename() process) into
    bs->auto_backing_file, if we know the former has been opened based on
    the latter.  This is only possible if no options modifying the backing
    file's behavior have been specified, though.  To simplify things, this
    patch only copies the filename from the backing file if no options have
    been specified for it at all.
    
    Furthermore, there are cases where an overlay is created by qemu which
    already contains a BDS's filename (e.g. in blockdev-snapshot-sync).  We
    do not need to worry about updating the overlay's bs->auto_backing_file
    there, because we actually wrote a post-bdrv_refresh_filename() filename
    into the image header.
    
    So all in all, there will be false negatives where (as of a future
    patch) bdrv_refresh_filename() will assume that the backing file differs
    from what was specified in the image header, even though it really does
    not.  However, these cases should be limited to where (1) the user
    actually did override something in the backing chain (e.g. by specifying
    options for the backing file), or (2) the user executed a QMP command to
    change some node's backing file (e.g. change-backing-file or
    block-commit with @backing-file given) where the given filename does not
    happen to coincide with qemu's idea of the backing BDS's filename.
    
    Then again, (1) really is limited to -drive.  With -blockdev or
    blockdev-add, you have to adhere to the schema, so a user cannot give
    partial "unimportant" options (e.g. by just setting backing.node-name
    and leaving the rest to the image header).  Therefore, trying to fix
    this would mean trying to fix something for -drive only.
    
    To improve on (2), we would need a full infrastructure to "canonicalize"
    an arbitrary filename (+ options), so it can be compared against
    another.  That seems a bit over the top, considering that filenames
    nowadays are there mostly for the user's entertainment.
    
    Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
    Reviewed-by: default avatarEric Blake <eblake@redhat.com>
    Reviewed-by: default avatarAlberto Garcia <berto@igalia.com>
    Message-id: 20190201192935.18394-5-mreitz@redhat.com
    Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
    998c2019
    History
    block: Add BDS.auto_backing_file
    Hanna Reitz authored
    
    If the backing file is overridden, this most probably does change the
    guest-visible data of a BDS.  Therefore, we will need to consider this
    in bdrv_refresh_filename().
    
    To see whether it has been overridden, we might want to compare
    bs->backing_file and bs->backing->bs->filename.  However,
    bs->backing_file is changed by bdrv_set_backing_hd() (which is just used
    to change the backing child at runtime, without modifying the image
    header), so bs->backing_file most of the time simply contains a copy of
    bs->backing->bs->filename anyway, so it is useless for such a
    comparison.
    
    This patch adds an auto_backing_file BDS field which contains the
    backing file path as indicated by the image header, which is not changed
    by bdrv_set_backing_hd().
    
    Because of bdrv_refresh_filename() magic, however, a BDS's filename may
    differ from what has been specified during bdrv_open().  Then, the
    comparison between bs->auto_backing_file and bs->backing->bs->filename
    may fail even though bs->backing was opened from bs->auto_backing_file.
    To mitigate this, we can copy the real BDS's filename (after the whole
    bdrv_open() and bdrv_refresh_filename() process) into
    bs->auto_backing_file, if we know the former has been opened based on
    the latter.  This is only possible if no options modifying the backing
    file's behavior have been specified, though.  To simplify things, this
    patch only copies the filename from the backing file if no options have
    been specified for it at all.
    
    Furthermore, there are cases where an overlay is created by qemu which
    already contains a BDS's filename (e.g. in blockdev-snapshot-sync).  We
    do not need to worry about updating the overlay's bs->auto_backing_file
    there, because we actually wrote a post-bdrv_refresh_filename() filename
    into the image header.
    
    So all in all, there will be false negatives where (as of a future
    patch) bdrv_refresh_filename() will assume that the backing file differs
    from what was specified in the image header, even though it really does
    not.  However, these cases should be limited to where (1) the user
    actually did override something in the backing chain (e.g. by specifying
    options for the backing file), or (2) the user executed a QMP command to
    change some node's backing file (e.g. change-backing-file or
    block-commit with @backing-file given) where the given filename does not
    happen to coincide with qemu's idea of the backing BDS's filename.
    
    Then again, (1) really is limited to -drive.  With -blockdev or
    blockdev-add, you have to adhere to the schema, so a user cannot give
    partial "unimportant" options (e.g. by just setting backing.node-name
    and leaving the rest to the image header).  Therefore, trying to fix
    this would mean trying to fix something for -drive only.
    
    To improve on (2), we would need a full infrastructure to "canonicalize"
    an arbitrary filename (+ options), so it can be compared against
    another.  That seems a bit over the top, considering that filenames
    nowadays are there mostly for the user's entertainment.
    
    Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
    Reviewed-by: default avatarEric Blake <eblake@redhat.com>
    Reviewed-by: default avatarAlberto Garcia <berto@igalia.com>
    Message-id: 20190201192935.18394-5-mreitz@redhat.com
    Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>