Re: Issue 90 Further Clarifications

On Fri, Nov 20, 2020 at 09:41:44 +0000, Daniel Berrange wrote: For new features though I strongly prefer if we no longer update the old code. Implement the new protocol only for -blockdev. The simplest way to probe for nfs protocol support is to use the following query string in virQEMUCapsQMPSchemaQueries[]: "blockdev-add/arg-type/+nfs" Looking at the @BlockdevOptionsNfs struct in qemu.git/qapi/block-core.json it seems that all properties were introduced at same time, so the check doesn't need to be more specific. I can provide more insight on how virQEMUCapsQMPSchemaQueries[] works if you are interested, but the above will work. Adding the NFS protocol itself is rather trivial because we use enum to string convertors which cause a compilation failure if you don't populate the strings, so adding the protocol type will be enough to figure out where. If you want to implement other properties of the nfs protocol driver such as @user or @group. I suggest you first send a RFC mail with your proposed XML addition for review before diving into the rng schema and XML/formatter parser. Looking at the options in @BlockdevOptionsNfs @user and @group seem a bit interesting. I'd not worry with the rest probably. I'm not going to outline why we've changed the old commit. You are implementing new code. You'll need to add a handler to qemuBlockStorageSourceGetBackendProps which converts the appropriate fields of virStorageSource to a virJSONValue object which maps to the qemu properties according to the @BlockdevOptionsNfs qemu struct. To verify that it's correct you can add a TEST_DISK_TO_JSON case to tests/qemublocktest.c (input files are in libvirt/tests/qemublocktestdata/xml2json/ ) where you provide a disk XML snippet and the output is what we'd use with qemu. Note that the 'onlytarget' boolean formats a string which is written into the qcow2 header file if you create an overlay/external snapshot, so it must not include any transient or authentication data. All of the above is QMP-schema validated so you'll be notified if it doesn't conform to qemu's monitor schema. You'll also need to implement a backing store string parser (that is the string which qemu writes to the overlay as noted above). The parser entry point is virStorageSourceParseBackingJSON and the backends for it are registered in virStorageSourceJSONDriverParser struct. The tests for the above are in tests/virstoragetest.c as TEST_BACKING_PARSE. Then depending on whether you actually want to add support for image creation (e.g. to support creating snapshots backed by the NFS storage directly) you then need to implement hanling in qemuBlockStorageSourceCreateGetStorageProps. You'll definitely see all the places that might need implementing once you add the new protocol entry to enum virStorageNetProtocol as we in many cases use switch statements with proper type where the compiler reminds you that you need to add the handling for the new value in the given patch. Since implementation of the qemu bits should be in a separate commit from the one adding the parser bits and thus the new enum, it's okay to just add the enum value to the swithc case and implement it later.