Re: [libvirt] [PATCH] virsh: fixed domdisplay command

On Tue, Jul 24, 2012 at 09:16:38AM +0200, Martin Kletzander wrote: > On 07/23/2012 09:42 PM, Daniel P. Berrange wrote: >> On Mon, Jul 23, 2012 at 02:25:23PM -0500, Doug Goldstein wrote: >>> On Mon, Jul 23, 2012 at 1:51 PM, Martin Kletzander <mkletzan(a)redhat.com&gt; wrote: >>>> The 'domdisplay' command didn't properly evaluate '--include-password' >>>> option. >>>> --- >>>> tools/virsh.c | 35 +++++++++++++++++++++++------------ >>>> 1 files changed, 23 insertions(+), 12 deletions(-) >>>> >>>> diff --git a/tools/virsh.c b/tools/virsh.c >>>> index 5888d6c..e0765ba 100644 >>>> --- a/tools/virsh.c >>>> +++ b/tools/virsh.c >>>> @@ -66,6 +66,7 @@ >>>> #include "virtypedparam.h" >>>> #include "intprops.h" >>>> #include "conf/virdomainlist.h" >>>> +#include "datatypes.h" >>> >>> Why exactly is this necessary? No new types are being used that aren't >>> used throughout this whole file. >> >> Indeed, datatypes.c is for the libvirt private details of >> the public objects. virsh is a public API consumer, so has >> no justification for using this include. >> > > I wasn't able to dereference virConnectPtr (conn->flags) without this > include, so gcc ended with an error. virsh has no business touching conn->flags at all. That is private libvirt data, not for public consumption.