[libvirt] [PATCH 08.5/12] qemu: Create a set of macros to handle setting block iotune values

[John Ferlan]

[...]

>>>
>>> Almost, but what I had in mind was to just keep one of the macros - be
>>> it SET_IOTUNE or SET_IOTUNE_FIELD or whatever the naming is - and then
>>> end up with a snippet like the one in the attached patch.
>>>
>>> Erik
>>>
>>> PS: Sorry for attaching a patch, but thunderbird's editor truly sucks
>>> (pasting from vim is a total nightmare)...big time...and I really have
>>> to switch to some normal mail client.
>>>
>>
>> If you 'temporarily' turn off line wrapping for t-bird, then cut-n-paste
>> isn't too bad, but attaching a patch was fine.
>>
> 
> Yeah, I could have done that, adjusting the message I sent along with
> the patch (I also tried selective wrap but that certainly didn't bring
> satisfactory results). Instead, I rage quit and thought that attaching
> the patch might not just be the worst possible way at all. Anyway, I'm
> starting to convince myself that spending several hours configuring mutt
> might as well turn out as a plausible solution of all email-related
> future problems than sticking with thunderbird after all.
> 

You're young, make the change... I think you have plenty of "tech
support" around you in Brno to get through all the 'gotchas'! Learning a
new email client could take me much longer ;-)


>> Suffice to say what you propose is where I initially started.  Then I
>> looked at the extended repetition of lines where the only difference was
>> "_max"/"_MAX" and "_max_length"/"_MAX_LENGTH" and felt it was better to
>> further collapse the way I did...
>>
>> In any case, see patch 10 for another instance of "parsing" 2 things at
>> a time (PARSE_IOTUNE) and patch 11 for adding the _max_length to that.
>> In fact the patch 11 shows my entrails with the commented out macros
>> (since removed from my branch).
>>
>> I chose not do the same in patch 4 (IOTUNE_ADD) or patch 9
>> (FORMAT_IOTUNE) because I didn't want to mess with the .args or .xml
>> output "adjustments" (e.g. output files).
>>
>> Personally I'd rather see the 3 variables close to each other rather
>> than hunting through the various output trying to see which are set. Way
>> too similarity that can "fool" the eyes.
>>
>> I'd prefer to keep the version of the macro I have since it doesn't
>> affect the output/visible order.
>>
>> John
>>
> 
> Well, I do see your point, I just have a different feeling about that.
> When I look at the qemuDomainSetBlockIoTune function, I see a bunch of
> variables defined, among which all the set_(bytes|iops) variables
> reside. Let's consider a massive function where you introduced a macro
> to get rid of all the duplicate code. What I expect from the macro is
> that I clearly see what parameters it takes so I can very simply
> identify which variables will be affected by the macro whatever it is
> trying to accomplish. By introducing another macro representing an
> additional meta layer of name processing, I fail to see what arguments
> will be eventually affected by the macro machinery at first glance.
> Your initial idea which was identical to my proposal makes it clear what
> variables will be affected by the macro and the only thing that needs to
> be handled is prefixing the enums with VIR_DOMAIN_BLOCK_IOTUNE. In my
> opinion, by visually grouping the related blocks of code can also lead
> to certain amount of cleanliness.
> Another thing, since in patch 4 and 9 you went the way I'm currently
> describing, I'm not a fan of treating essentially the same thing
> differently.
> 

I flipped a coin and it came up heads, which means I'll adjust this code
to be more like what you propose. I'll also likewise adjust patch 10 and
11 to PARSE_IOTUNE each variable - just to keep things consistent.

Now if I can only work through the RPC issue I put in a reply to .0 <sigh>

Tks -

John