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. > > + DO_TEST_SET("<bandwidth>" > + " <inbound average='1' peak='2' floor='3' burst='4'/>" > + " <outbound average='5' peak='6' burst='7'/>" > + "</bandwidth>", > + "/sbin/tc qdisc del dev eth0 root\n" > + "/sbin/tc qdisc del dev eth0 ingress\n" > + "/sbin/tc qdisc add dev eth0 root handle 1: htb default 1\n" I'm finding it a bit hard to visually see the argument separation. I would have written it: DO_TEST_SET(("<bandwidth" " <inbound .../>" " <outbound .../>" "</bandwidth"), ("/sbin/tc qdisc del dev eth0 root\n" "/sbin/tc ..."...) to draw a bit more of a visual clue where the string concats are divided into separate arguments. But that's cosmetic. ACK to this patch, once we figure out whether patch 3 is the ideal solution to the virCommand dry-run problem. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list