Re: [PATCH 4/4] virnetdevbandwidthtest: Introduce testVirNetDevBandwidthSet

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 24.01.2014 23:12, Eric Blake wrote:
> On 01/23/2014 06:44 AM, Michal Privoznik wrote:
>> The test tries to set some QoS limits and check if the commands
>> that are actually executed are the expected ones.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
>> ---
>>  tests/virnetdevbandwidthtest.c | 70 ++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 70 insertions(+)
> 
> Hmm - the idea of making virCommand have a dry-run mode might be useful
> in other testsuites.  Rather than mock things up with LD_PRELOAD, would
> it be worth adding an internal entry point to vircommand.c where we can
> pass in a file name from the testsuite code; if that file name is set,
> we dump the command name to the file instead of executing the command.
> While LD_PRELOAD hacks are nice, it would be even nicer to have the
> reusable testing framework not depend on a Linux-only solution.  That is
> more against 3/4, although if you do go with that approach, this patch
> might be impacted on calling into the internal hook to register the
> dry-run filename to write into.

Makes sense. However, it will require to have a global variable to
indicate that any virCommand is not to be executed but rather dry-run.
We don't want to modify virCommandNew* I assume - hence the global
variable. So we need two internal APIs then:

virCommandSetDryRun(const char *file);
virCommandCancelDryRun();

The first one just sets the global variable, while the other one un-sets
it. Perhaps the latter can be merged into the former:

virCommandSetDryRun(NULL);

but that's just cosmetics. Let me respin the 3/4 and 4/4. Meanwhile, I'm
pushing the first two patches with all the small nits you've pointed out
fixed. Thanks so far!

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]