On 02/27/2013 05:28 AM, John Ferlan wrote: > On 02/26/2013 07:54 PM, Eric Blake wrote: >> On 02/26/2013 07:20 AM, John Ferlan wrote: >> >> I would have put '---' here, since... >> > > The information was only in the email not part of the git history. > In the future I'll remember to put that after the '---'. It makes the most difference on projects where you don't have push rights, because the maintainer with push rights will use 'git am' to snarf in your email message, which discards comments after ---. Once you have push rights yourself, 'git am' is no longer involved in the loop, so there is nothing in the workflow to strip the comments, at which point it doesn't matter where you injected the comments into your email, other than the consistency factor. At any rate, it's always nice to know how to make the most use of a workflow that simplifies review time, which includes being consistent by injecting your email-only comments after ---. > > > Here's the differences from the v3 to what seems to be a happy medium. > > The virConnectNum* functions are still used just to "show" they exist > and how to use them. There's a comment before the usage. > > > > diff --git a/examples/hellolibvirt/hellolibvirt.c b/examples/hellolibvirt/hellol > index 335a75e..26dd67f 100644 > --- a/examples/hellolibvirt/hellolibvirt.c > +++ b/examples/hellolibvirt/hellolibvirt.c > @@ -91,8 +91,14 @@ static int > showDomains(virConnectPtr conn) > { > int ret = 0, i, numNames, numInactiveDomains, numActiveDomains; > - char **nameList = NULL; > - > + int flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE | > + VIR_CONNECT_LIST_DOMAINS_INACTIVE; Technically, a flags of 0 will give the same result (as the combination of ACTIVE|INACTIVE covers all domains); but this is reasonable for the example program (easier to see what to modify to get just half the set). > + virDomainPtr *nameList = NULL; > + * the current call. A domain could be started or stopped and any > + * assumptions made purely on these return values could result in > + * unexpected results */ > numActiveDomains = virConnectNumOfDomains(conn); > if (numActiveDomains == -1) { > ret = 1; > + /* Return a list of all active and inactive domains. Using this API > + * instead of virConnectListDomains() and virConnectListDefinedDomains() > + * is preferred since it "solves" an inherit race between separated API > + * calls if domains are started or stopped between calls */ > + numNames = virConnectListAllDomains(conn, > + &nameList, > + flags); > + for (i = 0; i < numNames; i++) { > + int active = virDomainIsActive(nameList[i]); > + printf(" %8s (%s)\n", > + virDomainGetName(nameList[i]), > + (active == 1 ? "active" : "non-active")); > /* must free the returned named per the API documentation */ > - free(*(nameList + i)); > + virDomainFree(nameList[i]); > } > + free(nameList); > > out: > - free(nameList); > return ret; ACK. > > This changes the output from: > > Attempting to connect to hypervisor > Connected to hypervisor at "qemu:///system" > Hypervisor: "QEMU" version: 0.32.656 > There are 0 active and 2 inactive domains > Inactive domains: > foo > bar > Disconnected from hypervisor > > > to > > Attempting to connect to hypervisor > Connected to hypervisor at "qemu:///system" > Hypervisor: "QEMU" version: 0.32.656 > There are 0 active and 2 inactive domains > foo (non-active) > bar (non-active) > Disconnected from hypervisor This might be worth putting in the commit message. At any rate, since this is a doc-only change, I'm comfortable with you pushing it before the 1.0.3 release. -- 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