On 09/23/2016 08:56 AM, John Ferlan wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1349898 > > Do a little housekeeping and minor adjustments to existing code, then > add the various "-length" options for the <iotune> code. > I realized while working on code adding another <iotune> attribute that I neglected to update/test 'virsh blkdeviotune $dom $device'. I had checked that 'virsh dumpxml $dom' displayed the new values, but brain cramped on blkdeviotune... In any case, a simple enough task one would think, right? Well as it turns out, no so simple. First off with these patches applied, I got the following: # virsh blkdeviotune $dom $disk error: Unable to get block I/O throttle parameters error: internal error: nparams too large # Uh oh... Long story short, there are now 19 parameters, but only 16 are allowed as a maximum (from src/remote/remote_protocol.x): const REMOTE_DOMAIN_BLOCK_IO_TUNE_PARAMETERS_MAX = 16; One would think "all" that would need to be done is change that 16 to say 24 (or 32 for more room). That led to a similar error.... Strange, until I discovered that the virTypedParamsDeserialize call in remoteDomainGetBlockIoTune used the wrong constant. So I changed that and fortunately my virsh works now - displaying what I want. BUT, again simple enough, right? Well not so fast. If I run an "older" virsh talking to the newer libvirtd, I get: # virsh blkdeviotune $dom $disk error: Unable to get block I/O throttle parameters error: Unable to decode message payload # Running that older virsh in a debugger, the failure occurs in xdr_remote_domain_get_block_io_tune_ret as soon as the 17th element is returned. I guess I'd expect that considering it's expecting at most 16 elements in it's xdr_array call. I guess part of me didn't expect this - the virsh client was able to allocate space for all 19 elements (due to the 0 nparams call being done first). I can understand the size of the returned data buffer not changing - that's not good, but if the client has done the right thing, then shouldn't that max param in the xdr_array call be the size of the buffer the client expects rather than the constant max? At least for the *Get* interfaces? So before I spent too much more time on digging on this - is this expected? (hopefully I've explained it well enough). Does this parameter count have to stay constant forever? It's too bad we didn't have the future vision that said there'd be more than 16 returned values. At this point I'm not sure what could be done from the (old) client perspective. If getting the old client error is OK or expected, then I can just go with the 24 or 32 for a BLOCK_IO_TUNE value. But I figured I'd put this out there and guage feedback. Tks - John FWIW: python client has similar phenomena > > John Ferlan (12): > docs: Fix typo in libvirt-domain.h parameter description > include: Update description for <iotune> max params > tests: Add blkdeviotune-max xml2xmltest > qemu: Convert from shorthand to longer throttling names > qemu: Adjust how supportMaxOptions is used. > include: Add new definitions for duration for bps/iops throttling > caps: Add new capability for the bps/iops throttling length > qemu: Add length for bps/iops throttling parameters to driver > conf: Add a formatting macro for all the blkiotune values > conf: Adjust the PARSE_IOTUNE macro > conf: Add support for blkiotune "_length" options > qemu: Add the length options to the iotune command line > > docs/formatdomain.html.in | 40 +++++- > docs/schemas/domaincommon.rng | 38 ++++++ > include/libvirt/libvirt-domain.h | 124 ++++++++++++++++-- > src/conf/domain_conf.c | 142 +++++++++++---------- > src/conf/domain_conf.h | 6 + > src/qemu/qemu_capabilities.c | 2 + > src/qemu/qemu_capabilities.h | 1 + > src/qemu/qemu_command.c | 96 ++++++-------- > src/qemu/qemu_driver.c | 106 ++++++++++++++- > src/qemu/qemu_monitor.c | 7 +- > src/qemu/qemu_monitor.h | 3 +- > src/qemu/qemu_monitor_json.c | 72 ++++++----- > src/qemu/qemu_monitor_json.h | 3 +- > .../caps_2.6.0-gicv2.aarch64.xml | 1 + > .../caps_2.6.0-gicv3.aarch64.xml | 1 + > tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 + > tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 + > tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 + > tests/qemumonitorjsontest.c | 17 ++- > .../qemuxml2argv-blkdeviotune-max-length.args | 34 +++++ > .../qemuxml2argv-blkdeviotune-max-length.xml | 65 ++++++++++ > .../qemuxml2argv-blkdeviotune-max.args | 10 +- > .../qemuxml2argv-blkdeviotune-max.xml | 14 +- > .../qemuxml2argv-blkdeviotune.args | 5 +- > tests/qemuxml2argvtest.c | 4 + > .../qemuxml2xmlout-blkdeviotune-max-length.xml | 1 + > .../qemuxml2xmlout-blkdeviotune-max.xml | 1 + > tests/qemuxml2xmltest.c | 2 + > 28 files changed, 616 insertions(+), 182 deletions(-) > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.args > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.xml > create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-blkdeviotune-max-length.xml > create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-blkdeviotune-max.xml > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list