Re: [Qemu-block] [PATCH v1 0/6] Report format specific info for LUKS block driver

[Eric Blake]

On 06/07/2016 04:11 AM, Daniel P. Berrange wrote:
> The 'qemu-img info' tool has ability to print format specific
> information, eg with qcow2 it reports two extra items:
> 
>   $ qemu-img info ~/VirtualMachines/demo.qcow2
>   image: /home/berrange/VirtualMachines/demo.qcow2
>   file format: qcow2
>   virtual size: 3.0G (3221225472 bytes)
>   disk size: 140K
>   cluster_size: 65536
>   Format specific information:
>       compat: 0.10
>       refcount bits: 16
> 
> 
> This is not currently wired up for the LUKS driver. This patch
> series adds that support so that we can report useful data about
> the LUKS volume such as the crypto algorithm choices, key slot
> usage and other volume metadata.
> 
> The first patch extends the crypto API to allow querying of the
> format specific metadata
> 
> The second patches extends the block API to allow the LUKS driver
> to report the format specific metadata.

Nice!


>         uuid: 6ddee74b-3a22-408c-8909-6789d4fa2594
>         slots:
>             [0]:
>                 active: true
>                 iters: 572706
>                 stripes: 4000
>                 key-offset: 8
>             [1]:
>                 active: false
>                 iters: 0
>                 stripes: 4000
>                 key-offset: 264
>             [2]:
>                 active: false
>                 iters: 0
>                 stripes: 4000
>                 key-offset: 520

Is it worth making iters/stripes optional, and present only when active
is true?  key-offset, on the other hand, should always be present.

> The remaining 4 patches are improvements to QAPI and the core
> block layer to fix a problem whereby struct fields are output
> in (apparently) random ordering. This is because the QAPI type
> is converted into a QObject for pretty-printing, thus throwing
> away any struct field ordering information.
> 
> To address this I created a new TextOutputVisitor which can
> directly pretty-print QAPI types. I then changed the code
> generator to create qapi_stringify_TYPENAME() methods for
> all QAPI types. Finally I changed the block layer over to
> use this stringify approach instead.

Umm, how much does that duplicate my work on adding the JSON pretty
printer, which DOES print JSON in the same order as the underlying QAPI
struct?  I still need to address Markus' latest reviews, but suspect we
may be able to combine efforts rather than duplicating.

> 
> I'm not sure if QAPI maintainers will find the idea of adding
> qapi_stringify_TYPENAME() methods desirable or not. It is of
> course valid to directly use the TextOutputVisitor from the
> block layer. I felt there might be some use in debugging to
> have a convenient qapi_stringify_TYPENAME() method around
> though - personally I often find it helpful to be able to
> easily dump an QAPI object or any QObject to a humand friendly
> format for debugging and the less code I need write to add
> this temporary debug output the better.

I agree with that sentiment, but being able to dump directly to JSON
without going through an intermediate QObject may already be what you want.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature