[libvirt] [RFC PATCH 05/30] qemu: block: Extract formatting of props for 'file' backend

[Peter Krempa]

On Fri, Apr 20, 2018 at 09:45:40 +0200, Kevin Wolf wrote:
> Am 19.04.2018 um 17:25 hat Peter Krempa geschrieben:
> > 'file' backend in qemu supports few more options than the current
> > implementation. Extract it so that changes don't pollute the code.
> > 
> > Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> > ---
> >  src/qemu/qemu_block.c | 16 +++++++++++++---
> >  1 file changed, 13 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> > index c0cf6a95ad..e7bd6c909d 100644
> > --- a/src/qemu/qemu_block.c
> > +++ b/src/qemu/qemu_block.c
> > @@ -974,6 +974,18 @@ qemuBlockStorageSourceGetSshProps(virStorageSourcePtr src)
> >  }
> > 
> > 
> > +static virJSONValuePtr
> > +qemuBlockStorageSourceGetFileProps(virStorageSourcePtr src)
> > +{
> > +    virJSONValuePtr ret = NULL;
> > +
> > +    ignore_value(virJSONValueObjectCreate(&ret,
> > +                                          "s:driver", "file",
> > +                                          "s:filename", src->path, NULL) < 0);
> > +    return ret;
> > +}
> > +
> > +
> >  /**
> >   * qemuBlockStorageSourceGetBackendProps:
> >   * @src: disk source
> > @@ -991,9 +1003,7 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src)
> >      case VIR_STORAGE_TYPE_BLOCK:
> >      case VIR_STORAGE_TYPE_FILE:
> >      case VIR_STORAGE_TYPE_DIR:
> > -        if (virJSONValueObjectCreate(&fileprops,
> > -                                     "s:driver", "file",
> > -                                     "s:filename", src->path, NULL) < 0)
> > +        if (!(fileprops = qemuBlockStorageSourceGetFileProps(src)))
> >              return NULL;
> 
> Preexisting, but I wonder whether 'driver': 'file' is correct for all
> these cases. I see that you handle _DIR separately with a vvfat function
> later in this series, but shouldn't _BLOCK be 'host_device' rather than
> 'file', too?

It should. I just did not notice it. If I try the block storage now I
get:

    {
      "iops_rd": 0,
      "detect_zeroes": "off",
      "image": {
        "virtual-size": 10737418240,
        "filename": "/dev/mapper/angien--storage-ble",
        "format": "host_device",
        "actual-size": 0,
        "dirty-flag": false
      },
      "iops_wr": 0,
      "ro": false,
      "node-name": "#block093",
      "backing_file_depth": 0,
      "drv": "host_device",
      "iops": 0,
      "bps_wr": 0,
      "write_threshold": 0,
      "encrypted": true,
      "bps": 0,
      "bps_rd": 0,
      "cache": {
        "no-flush": false,
        "direct": true,
        "writeback": true
      },
      "file": "/dev/mapper/angien--storage-ble",
      "encryption_key_missing": false
    }

Is there a special need to use 'host_cdrom' explicitly if the CDROM
drive is used? That would complicate things since we don't know when
that will happen.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180420/6926428d/attachment-0001.sig>