Re: [PATCH] Re: Proposal: Check availability of driver calls (repost)

Daniel Veillard wrote:
>> +typedef enum {
>> +    /* Driver supports V1-style virDomainMigrate, ie. domainMigratePrepare/
>> +     * domainMigratePerform/domainMigrateFinish.
>> +     */
>> +    VIR_DRV_FEATURE_MIGRATION_V1 = 1,
>> +
>> +    /* Driver is not local. */
>> +    VIR_DRV_FEATURE_REMOTE = 2,
>> +} virDrvFeature;
> 
>   Probably best done with defines than enums, as you want to be able to compose
> them the fact it comes from a type doesn't help, and I have learnt the hard way
> that enums sucks in C in general.

What in particular?  I'm not particularly concerned either way since 
enums are basically just as unsafe as #defines in C, but it is nice for 
the library user to be able to connect the argument prototype 
("virDrvFeature feature") to the list of permitted types.

>> +typedef int
>> +    (*virDrvSupportsFeature) (virConnectPtr conn, virDrvFeature feature);
> 
>  I would rather use , int features) where you OR the features, allows
> to know in one call if you get what you want or not.

In the case where you've got multiple different migration possibilities 
(VIR_DRV_FEATURE_MIGRATE_V1 & VIR_DRV_FEATURE_MIGRATE_V2) then this 
saves you one remote call in the legacy case (VIR_DRV_FEATURE_MIGRATE_V2 
not supported so we have to do a second remote call to check 

Re: [PATCH] Re: Proposal: Check availability of driver calls (repost)

On Wed, Aug 01, 2007 at 02:45:57PM +0100, Richard W.M. Jones wrote:
> Daniel Veillard wrote:
> >>+typedef enum {
> >>+    /* Driver supports V1-style virDomainMigrate, ie. 
> >>domainMigratePrepare/
> >>+     * domainMigratePerform/domainMigrateFinish.
> >>+     */
> >>+    VIR_DRV_FEATURE_MIGRATION_V1 = 1,
> >>+
> >>+    /* Driver is not local. */
> >>+    VIR_DRV_FEATURE_REMOTE = 2,
> >>+} virDrvFeature;
> >
> >  Probably best done with defines than enums, as you want to be able to 
> >  compose
> >them the fact it comes from a type doesn't help, and I have learnt the 
> >hard way
> >that enums sucks in C in general.
> 
> What in particular?  I'm not particularly concerned either way since 
> enums are basically just as unsafe as #defines in C, but it is nice for 
> the library user to be able to connect the argument prototype 
> ("virDrvFeature feature") to the list of permitted types.
> 
> >>+typedef int
> >>+    (*virDrvSupportsFeature) (virConnectPtr conn, virDrvFeature 
> >>feature);
> >
> > I would rather use , int features) where you OR the features, allows
> >to know in one call if you get what you want or not.

Re: Re: [PATCH] Re: Proposal: Check availability of driver calls (repost)

On Wed, Aug 01, 2007 at 10:19:42AM -0400, Daniel Veillard wrote:
> On Wed, Aug 01, 2007 at 02:45:57PM +0100, Richard W.M. Jones wrote:
> > Daniel Veillard wrote:
> > >>+typedef enum {
> > >>+    /* Driver supports V1-style virDomainMigrate, ie. 
> > >>domainMigratePrepare/
> > >>+     * domainMigratePerform/domainMigrateFinish.
> > >>+     */
> > >>+    VIR_DRV_FEATURE_MIGRATION_V1 = 1,
> > >>+
> > >>+    /* Driver is not local. */
> > >>+    VIR_DRV_FEATURE_REMOTE = 2,
> > >>+} virDrvFeature;
> > >
> > >  Probably best done with defines than enums, as you want to be able to 
> > >  compose
> > >them the fact it comes from a type doesn't help, and I have learnt the 
> > >hard way
> > >that enums sucks in C in general.
> > 
> > What in particular?  I'm not particularly concerned either way since 
> > enums are basically just as unsafe as #defines in C, but it is nice for 
> > the library user to be able to connect the argument prototype 
> > ("virDrvFeature feature") to the list of permitted types.
> > 
> > >>+typedef int
> > >>+    (*virDrvSupportsFeature) (virConnectPtr conn, virDrvFeature 
> > >>feature);
> > >
> > > I would rather use , int features) where you OR the features, allows

Re: [PATCH] Re: Proposal: Check availability of driver calls (repost)

Daniel Veillard wrote:
>   You seems to think it's useful only for migrate. I want that to be
> usable for a lot more.

Examples being ...?  I want to know if there's a real example where a 
coordinating function would like to query > 1 feature of the underlying 
driver, and if doing so save signicant numbers of round trips in the 
remote case.

> You talk to a remote server, you want to know if
> it supports any of the entry point in the driver table, how do you do this ?

I don't claim that it can do this.  In the original design I looked at 
how one might do this and concluded it was pretty difficult.  You can't 
just use indexes into the driver structure because they change depending 
on architecture and packing rules[1].

>> On the other hand, it complicates the interface.  You need to return an 
>> array rather than a single int.  (OK, so you can return a bit array, but 
>> now the feature list had better always be <= 32 entries long).  And in 
>> the case where someone queries VIR_DRV_FEATURE_MIGRATE_V1 & 
>> VIR_DRV_FEATURE_REMOTE you need to have two different drivers answering 
>> a single request.
>>
>> I think this complicates things unnecessarily ...
> 
>   I think this would be a waste to design it with a single narrowly focused
> usage in mind, when it can be far more generally useful.

and Dan Berrange wrote:

Re: [PATCH] check file format in virsh attach/detach-device

Hi Daniel, Rich

Thanks for reviewing and suggestion.

I adopt (b) in consideration Daniel's suggestion.
 --> Error message exists already.

This patch fixes virParseXMLDevice() for attach-device
and virDomainXMLDevID() for detach-device.

Thanks,
Masayuki Sunou.

----------------------------------------------------------------------
Index: src/xml.c
===================================================================
RCS file: /data/cvs/libvirt/src/xml.c,v
retrieving revision 1.86
diff -u -p -r1.86 xml.c
--- src/xml.c	31 Jul 2007 14:27:12 -0000	1.86
+++ src/xml.c	2 Aug 2007 05:41:21 -0000
 <at>  <at>  -1400,8 +1400,10  <at>  <at>  virParseXMLDevice(virConnectPtr conn, ch
     xml = xmlReadDoc((const xmlChar *) xmldesc, "domain.xml", NULL,
                      XML_PARSE_NOENT | XML_PARSE_NONET |
                      XML_PARSE_NOERROR | XML_PARSE_NOWARNING);
-    if (xml == NULL)
+    if (xml == NULL) {
+        virXMLError(conn, VIR_ERR_XML_ERROR, NULL, 0);
         goto error;
+    }

Re: [PATCH] check the file name for --log

Hi, Dan

Then This kind of Warning is acceptable?

Thanks
Atsushi SAKAI

"Daniel P. Berrange" <berrange <at> redhat.com> wrote:

> On Tue, Jul 31, 2007 at 08:12:56PM +0900, Atsushi SAKAI wrote:
> > Hi,
> > 
> > This patch fixes the trouble if execute virsh with 
> > virsh --log --debug.
> > In this case FILE "--debug" is generated and 
> > --debug is off. 
> > 
> > This patch treat as error return 
> > if it uses the hyphen "-" at the head of log file name.
> 
> NACK. The code is already doing exactly what the user asked. The --log
> flag takes an argument and you are supplied --debug as that argument.
> Therefore it creates the file --debug. This is fine.
> 
> Dan.
> -- 
> |=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
> |=-           Perl modules: http://search.cpan.org/~danberr/              -=|
> |=-               Projects: http://freshmeat.net/~danielpb/               -=|
> |=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 

Re: [PATCH] check file format in virsh attach/detach-device

On Thu, Aug 02, 2007 at 03:01:07PM +0900, Masayuki Sunou wrote:
> Hi Daniel, Rich
> 
> Thanks for reviewing and suggestion.
> 
> I adopt (b) in consideration Daniel's suggestion.
>  --> Error message exists already.
> 
> This patch fixes virParseXMLDevice() for attach-device
> and virDomainXMLDevID() for detach-device.

  Looks fine to me, I applied it after just a couple of cosmetic changes.

   thanks a lot, it's in CVS !

Daniel

--

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard <at> redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

ANNOUNCE: OCaml bindings 0.3.1.0 released

http://et.redhat.com/~rjones/ocaml-libvirt/

This removes use of the dangerous vir{Domain,Network}GetConnect 
functions and makes the relationship between domains, networks and 
connections explicit to the garbage collector.

I've also added support for domain migration.

mlvirtmanager displays the hostname when connected to multiple remote 
hosts (see screenshot attached).

Rich.

--

-- 
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom.  Registered in
England and Wales under Company Registration No. 03798903

http://et.redhat.com/~rjones/ocaml-libvirt/

This removes use of the dangerous vir{Domain,Network}GetConnect 
functions and makes the relationship between domains, networks and 
connections explicit to the garbage collector.

I've also added support for domain migration.

ANNOUNCE: Virt-top - a top-like utility for displaying virtualization stats

I'm pleased to announce the first release of virt-top, which is a 
top-like utility for displaying virtualization stats.

It aims to look and feel very much like regular 'top', so as to be as 
familiar as possible for systems administrators.  You can also use it as 
a pleasant replacement for xentop.

It uses libvirt, so can display stats across a variety of different 
hypervisors and virtualization systems (not just Xen, although that is 
where the testing has gone so far).

   http://et.redhat.com/~rjones/virt-top/

The license is a combination of LGPL (for the library) and GPL (for the 
virt-top program).

Current status
--------------

You can view domains and use familiar keys like 'P'/'M'/... to sort by 
processor/memory/..., and 'd'/'s' to set the delay between updates. 
Also some common top command-line options are implemented.  The man page 
is here: http://et.redhat.com/~rjones/virt-top/virt-top.txt

There are a variety of source and binary RPMs available for Fedora 
users.  I don't yet have a working Debian/Ubuntu package, but will have 
a go at making one tomorrow.

The next thing I'll be working on is showing virtual and physical CPU 
usage of guests.  After that I'm hoping to discuss extensions to libvirt 

Re: ANNOUNCE: Virt-top - a top-like utility for displaying virtualization stats

On Mon, Aug 06, 2007 at 10:15:48PM +0100, Richard W.M. Jones wrote:
> There are a few data collection artifacts which need to be investigated. 
>  In particular, %CPU sometimes goes over 100%.  Obviously accurate data 
> collection is an important goal for this tool.

AFAICT it is impossible to stop it going over 100%  - we have same issue 
in  virt-manager when calculating CPU usage. To caculate the % usage you 
are looking at the differential between CPU time from two calls to 
virDomainGetInfo vs the time period between two gettimeofday() calls.
If the time delta between your gettimeofday() calls does not exactly 
match the time delta between your virDomainGetInfo() calls, and the 
domain you're measuring is very active then you can end up calcuating 
> 100% occassionally. Not by much - typically < 1% over, but I don't
really see any way around it.

Regards,
Dan.
--

-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=|