Bug 1442344

Hi yisun, what about nbd backend?

(In reply to Han Han from comment #3)
> Hi yisun, what about nbd backend?

Doesn't work

root@localhost ~  ## virsh domblklist vm1
Target     Source
------------------------------------------------
vda        /var/lib/libvirt/images/rhel7.3.qcow2
vdb        -
vdc        libvirt-pool/rbd1.img
vdd        /var/lib/libvirt/images/1.qcow2

root@localhost ~  ## virsh domblkinfo vm1 vdb
error: internal error: missing storage backend for network files using nbd protocol

This was caused by commit [1]. The code previously updated the physical size only for block devices and thus would rely on data from qemu in other cases.

With this change virStorageSourceUpdatePhysicalSize tries to open the storage source also for networked storage protocols where we did not implement the handlers yet.

Since the failure of qemuDomainStorageUpdatePhysical is fatal in case of qemuDomainBlockGetInfo, this fails for all storage protocols which are not implemented.

Additionally commit [2] made it more likely to call the function.

We need to fix this by skipping the call if qemuDomainStorageUpdatePhysical and the underlying functions are not implemented and return incomplete data as we would until now.

[1]
commit c5f6151390ff0a8e65014172bb8c0a8d312c3353
Author: John Ferlan <jferlan>
Date:   Thu Dec 1 16:19:26 2016 -0500

    util: Introduce virStorageSourceUpdatePhysicalSize
    
    Commit id '8dc27259' introduced virStorageSourceUpdateBlockPhysicalSize
    in order to retrieve the physical size for a block backed source device
    for an active domain since commit id '15fa84ac' changed to use the
    qemuMonitorGetAllBlockStatsInfo and qemuMonitorBlockStatsUpdateCapacity
    API's to (essentially) retrieve the "actual-size" from a 'query-block'
    operation for the source device.
    
    However, the code only was made functional for a BLOCK backing type
    and it neglected to use qemuOpenFile, instead using just open. After
    the open the block lseek would find the end of the block and set the
    physical value, close the fd and return.
    
    Since the code would return 0 immediately if the source device wasn't
    a BLOCK backed device, the physical would be displayed incorrectly,
    such as follows in domblkinfo for a file backed source device:
    
    Capacity:       1073741824
    Allocation:     0
    Physical:       0
    
    This patch will modify the algorithm to get the physical size for other
    backing types and it will make use of the qemuDomainStorageOpenStat
    helper in order to open/stat the source file depending on its type.
    The qemuDomainGetStatsOneBlock will no longer inhibit printing errors,
    but it will still ignore them leaving the physical value set to 0.

[2]
commit b9b1aa6392e058b9c3814ea391943bc3be18ccba
Author: John Ferlan <jferlan>
Date:   Mon Dec 12 17:12:09 2016 -0500

    qemu: Adjust qemuDomainGetBlockInfo data for sparse backed files
    
    According to commit id '0282ca45a' the 'physical' value should
    essentially be the last offset of the image or the host physical
    size in bytes of the image container. However, commit id '15fa84ac'
    refactored the GetBlockInfo to use the same returned data as the
    GetStatsBlock API for an active domain. For the 'entry->physical'
    that would end up being the "actual-size" as set through the
    qemuMonitorJSONBlockStatsUpdateCapacityOne (commit '7b11f5e5').
    Digging deeper into QEMU code one finds that actual_size is
    filled in using the same algorithm as GetBlockInfo has used for
    setting the 'allocation' field when the domain is inactive.
    
    The difference in values is seen primarily in sparse raw files
    and other container type files (such as qcow2), which will return
    a smaller value via the stat API for 'st_blocks'. Additionally
    for container files, the 'capacity' field (populated via the
    QEMU "virtual-size" value) may be slightly different (smaller)
    in order to accomodate the overhead for the container. For
    sparse files, the state 'st_size' field is returned.
    
    This patch thus alters the allocation and physical values for
    sparse backed storage files to be more appropriate to the API
    contract. The result for GetBlockInfo is the following:
    
     capacity: logical size in bytes of the image (how much storage
               the guest will see)
     allocation: host storage in bytes occupied by the image (such
                 as highest allocated extent if there are no holes,
                 similar to 'du')
     physical: host physical size in bytes of the image container
               (last offset, similar to 'ls')
    
    NB: The GetStatsBlock API allows a different contract for the
    values:
    
     "block.<num>.allocation" - offset of the highest written sector
                                as unsigned long long.
     "block.<num>.capacity" - logical size in bytes of the block device
                              backing image as unsigned long long.
     "block.<num>.physical" - physical size in bytes of the container
                              of the backing image as unsigned long long.

Fixed upstream:

commit 9f16bb7386939c400a7522d2489245f1e6831bae
Author: Peter Krempa <pkrempa>
Date:   Thu Apr 20 11:16:42 2017 +0200

    qemu: Don't fail if physical size can't be updated in qemuDomainGetBlockInfo
    
    Since commit c5f6151390 qemuDomainBlockInfo tries to update the
    "physical" storage size for all network storage and not only block
    devices.
    
    Since the storage driver APIs to do this are not implemented for certain
    storage types (RBD, iSCSI, ...) the code would fail to retrieve any data
    since the failure of qemuDomainStorageUpdatePhysical is fatal.
    
    Since it's desired to return data even if the total size can't be
    updated we need to ignore errors from that function and return plausible
    data.

Verified on:
qemu-kvm-rhev-2.9.0-3.el7.x86_64
libvirt-3.2.0-4.virtcov.el7.x86_64

As we know, the network protocols that qemu-kvm-rhev and libvirt both suppport are: iscsi, nbd, rbd, gluster, https.
steps:
1. Start a VM with iscsi, nbd, rbd, gluster, https storage:
   <disk type='network' device='disk'>
      <driver name='qemu' type='raw'/>
      <source protocol='iscsi' name='iqn.2016-12.com.virttest:emulated-iscsi-noauth.target0/0'>
        <host name='10.66.5.92' port='3260'/>
      </source>
      <backingStore/>
      <target dev='sdb' bus='scsi'/>
      <alias name='scsi0-0-0-1'/>
      <address type='drive' controller='0' bus='0' target='0' unit='1'/>
    </disk>
    <disk type='network' device='disk'>
      <driver name='qemu' type='raw'/>
      <source protocol='nbd' name='nbd_raw'>
        <host name='10.66.5.92' port='10803'/>
      </source>
      <backingStore/>
      <target dev='sdc' bus='scsi'/>
      <alias name='scsi0-0-0-2'/>
      <address type='drive' controller='0' bus='0' target='0' unit='2'/>
    </disk>
    <disk type='network' device='disk'>
      <driver name='qemu' type='raw'/>
      <source protocol='rbd' name='rbd/rbd.img'>
        <host name='10.73.75.52'/>
      </source>
      <backingStore/>
      <target dev='sdd' bus='scsi'/>
      <alias name='scsi0-0-0-3'/>
      <address type='drive' controller='0' bus='0' target='0' unit='3'/>
    </disk>
    <disk type='network' device='disk'>
      <driver name='qemu' type='qcow2'/>
      <source protocol='gluster' name='gluster-vol1/16572.qcow2'>
        <host name='10.66.5.92'/>
      </source>
      <backingStore/>
      <target dev='sde' bus='scsi'/>
      <alias name='scsi0-0-0-4'/>
      <address type='drive' controller='0' bus='0' target='0' unit='4'/>
    </disk>
    <disk type='network' device='cdrom'>
      <driver name='qemu' type='raw'/>
      <source protocol='https' name='/pub/fedora/linux/releases/25/Workstation/x86_64/iso/Fedora-Workstation-netinst-x86_64-25-1.3.iso'>
        <host name='download.fedoraproject.org' port='443'/>
      </source>
      <backingStore/>
      <target dev='sdf' bus='scsi'/>
      <readonly/>
      <alias name='scsi0-0-0-5'/>
      <address type='drive' controller='0' bus='0' target='0' unit='5'/>
    </disk>

2. Check their domblkinfo
# for dev in sd{b..f};do virsh domblkinfo Q $dev;done
Capacity:       32212254720
Allocation:     32212254720
Physical:       32212254720

Capacity:       10737418240
Allocation:     10737418240
Physical:       10737418240

Capacity:       5368709120
Allocation:     5368709120
Physical:       5368709120

Capacity:       10737418240
Allocation:     1356791808
Physical:       1356791808

Capacity:       486539264
Allocation:     486539264
Physical:       486539264

All results are expected. Bug fixed.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHEA-2017:1846

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHEA-2017:1846