[libvirt] [PATCH 09/10 v5] list: Use virConnectListAllStoragePools in virsh

Osier Yang jyang at redhat.com
Thu Sep 6 13:53:08 UTC 2012


On 2012年09月06日 01:17, Laine Stump wrote:
>
> On 09/05/2012 12:52 PM, Eric Blake wrote:
>> On 09/05/2012 12:36 AM, Osier Yang wrote:
>>> +++ b/tools/virsh-pool.c
>>> @@ -36,6 +36,7 @@
>>>   #include "memory.h"
>>>   #include "util.h"
>>>   #include "xml.h"
>>> +#include "conf/storage_conf.h"
>> I'm not sure if virsh is supposed to be able to use conf/*.h files;
>> you're not the first offender, but the more we do this, the more we are
>> admitting that our public API is insufficient.  I'm wondering if we
>> should move the filter group constants into libvirt.h, and make them
>> part of the public API...

For the constants, yeah, we have to move them to public if not want to
include the conf headers in client side.

>
> Yes. (or whatever it takes to not use non-public .h files in virsh).
> virsh should only use the public libvirt API; if it needs something
> that's private to libvirt, either that piece of code should be
> rewritten, or the public API should be enhanced.

Agreed for sure, I didn't notice that when creating the patches though.

  (But, as you say, Osier
> isn't the first offender, so it's okay to let this temporarily slip by).

Thanks, I will push the set, and see if can I create a later patch
to fix that.

>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list




More information about the libvir-list mailing list