[libvirt] PATCH [0/2]: Deprecate conn, dom, net virterror fields
This is a pair of patches which deprecate the conn, dom and net fields in the virterror structure. Programs which use these fields will get a warning (if compiled under gcc anyway): foo.c:123: warning: 'dom' is deprecated (declared at /usr/include/libvirt/virterror.h:81) The fields themselves are still present and still set, so the ABI isn't changed. Now the patch is quite a lot more involved than you might think for such a simple change. Part of the problem is that we want to allow libvirt itself to modify these fields without generating a warning. Therefore the first part of the patch has to refactor all code within libvirt which includes "libvirt.h" or "virterror.h" (the public headers) so that instead this code just includes "internal.h". "internal.h" already includes the public headers. This allows "internal.h" to undefine the deprecation macro. Of course nothing is quite so simple. We have two "internal.h" files, so I renamed the one in the qemud directory to "qemud.h". I also added something in the HACKING file, stating how *.c files within libvirt should include header files. The second part of the patch actually adds the deprecation to the fields in virterror, and hence is small and simple. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
One thing to note about this patch is that qemud/internal.h has been renamed to qemud/qemud.h. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top
On Thu, May 22, 2008 at 06:42:21PM +0100, Richard W.M. Jones wrote:
One thing to note about this patch is that qemud/internal.h has been renamed to qemud/qemud.h.
ACK, this all looks very sensible. I should never have named that file qemud/internal.h in the first place. Dunno what i was thinking ! Dan. -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
"Richard W.M. Jones" <rjones@redhat.com> wrote:
One thing to note about this patch is that qemud/internal.h has been renamed to qemud/qemud.h.
Nice clean up! ACK
Index: HACKING =================================================================== ... \ No newline at end of file
Good to fix, but not big deal.
Index: qemud/qemud.c =================================================================== -#include "../src/util.h" -#include "../src/remote_internal.h" -#include "../src/conf.h" +#include "util.h" +#include "remote_internal.h" +#include "conf.h"
Good to see the "../src/" prefix being removed everywhere!
-- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
ACK. I've just noticed the QEMU driver does ever fill in the dom or net fields at all, and we know the remote driver doesn't either. Dan -- |: Red Hat, Engineering, Boston -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
"Richard W.M. Jones" <rjones@redhat.com> wrote:
Index: include/libvirt/libvirt.h.in =================================================================== RCS file: /data/cvs/libvirt/include/libvirt/libvirt.h.in,v retrieving revision 1.48 diff -u -r1.48 libvirt.h.in --- include/libvirt/libvirt.h.in 15 May 2008 06:12:32 -0000 1.48 +++ include/libvirt/libvirt.h.in 22 May 2008 16:56:58 -0000 @@ -21,6 +21,14 @@ extern "C" { #endif
+#ifndef VIR_DEPRECATED +#ifdef __GNUC__ +#define VIR_DEPRECATED __attribute__((deprecated)) +#else +#define VIR_DEPRECATED /* nothing */ +#endif +#endif /* VIR_DEPRECATED */
This feature appears to have been introduced in gcc-3.1, so define away for earlier versions. Also, it's better for name-space cleanliness to use the __...__ form: #ifndef VIR_DEPRECATED /* The feature is present in gcc-3.1 and newer. */ # if __GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ >= 1) # define VIR_DEPRECATED __attribute__((__deprecated__)) # else # define VIR_DEPRECATED /* nothing */ # endif #endif /* VIR_DEPRECATED */
Hi, Rich Your code seems to make compilation trouble. http://builder.virt-manager.org/module-libvirt--devel.html http://git.et.redhat.com/?p=libvirt.git;a=commitdiff;h=fc949fa7b7e043401f4c5... Thanks Atsushi SAKAI "Richard W.M. Jones" <rjones@redhat.com> wrote:
This is a pair of patches which deprecate the conn, dom and net fields in the virterror structure.
Programs which use these fields will get a warning (if compiled under gcc anyway):
foo.c:123: warning: 'dom' is deprecated (declared at /usr/include/libvirt/virterror.h:81)
The fields themselves are still present and still set, so the ABI isn't changed.
Now the patch is quite a lot more involved than you might think for such a simple change. Part of the problem is that we want to allow libvirt itself to modify these fields without generating a warning.
Therefore the first part of the patch has to refactor all code within libvirt which includes "libvirt.h" or "virterror.h" (the public headers) so that instead this code just includes "internal.h". "internal.h" already includes the public headers. This allows "internal.h" to undefine the deprecation macro.
Of course nothing is quite so simple. We have two "internal.h" files, so I renamed the one in the qemud directory to "qemud.h".
I also added something in the HACKING file, stating how *.c files within libvirt should include header files.
The second part of the patch actually adds the deprecation to the fields in virterror, and hence is small and simple.
Rich.
-- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, May 27, 2008 at 05:39:16PM +0900, Atsushi SAKAI wrote:
Hi, Rich
Your code seems to make compilation trouble. http://builder.virt-manager.org/module-libvirt--devel.html
Thanks, taking a look now ... Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/
On Tue, May 27, 2008 at 09:40:55AM +0100, Richard W.M. Jones wrote:
On Tue, May 27, 2008 at 05:39:16PM +0900, Atsushi SAKAI wrote:
Hi, Rich
Your code seems to make compilation trouble. http://builder.virt-manager.org/module-libvirt--devel.html
Thanks, taking a look now ...
I've checked in a patch which fixes this. The patch is attached. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/
Hi, Rich It seems and compiles fine for me. +1 Thanks Atsushi SAKAI "Richard W.M. Jones" <rjones@redhat.com> wrote:
On Tue, May 27, 2008 at 09:40:55AM +0100, Richard W.M. Jones wrote:
On Tue, May 27, 2008 at 05:39:16PM +0900, Atsushi SAKAI wrote:
Hi, Rich
Your code seems to make compilation trouble. http://builder.virt-manager.org/module-libvirt--devel.html
Thanks, taking a look now ...
I've checked in a patch which fixes this. The patch is attached.
Rich.
-- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/
On Thu, May 22, 2008 at 06:40:57PM +0100, Richard W.M. Jones wrote:
This is a pair of patches which deprecate the conn, dom and net fields in the virterror structure.
Programs which use these fields will get a warning (if compiled under gcc anyway):
foo.c:123: warning: 'dom' is deprecated (declared at /usr/include/libvirt/virterror.h:81)
The fields themselves are still present and still set, so the ABI isn't changed.
Now the patch is quite a lot more involved than you might think for such a simple change. Part of the problem is that we want to allow libvirt itself to modify these fields without generating a warning.
Therefore the first part of the patch has to refactor all code within libvirt which includes "libvirt.h" or "virterror.h" (the public headers) so that instead this code just includes "internal.h". "internal.h" already includes the public headers. This allows "internal.h" to undefine the deprecation macro.
Of course nothing is quite so simple. We have two "internal.h" files, so I renamed the one in the qemud directory to "qemud.h".
I also added something in the HACKING file, stating how *.c files within libvirt should include header files.
The second part of the patch actually adds the deprecation to the fields in virterror, and hence is small and simple.
As indicated on Friday over IRC (my mail was down !) Okay, fine by me. it's a bit frustrating to not be able to give the full extent of the informations we have available because we can't just do the ref counting right, but it's the best way, better to deprecate than risk having the error handling crash Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
participants (5)
-
Atsushi SAKAI -
Daniel P. Berrange -
Daniel Veillard -
Jim Meyering -
Richard W.M. Jones