[Libvir] [PATCH] change a Disk/Nic of inactive domain
Hi The libvirt on Xen-3.03 does not have the function that add/change/delete a Disk/NIC of inactive domain. So, I made a patch which enabled add/change/delete of Disk/NIC of inactive domain. This patch will be able to change "disk" or "vif" parameter in configuration file with "attach-disk","attach-interface", "attach-device", "detach-disk", "detach-interface" and "detach-device" command, like "setmem" or "setvcpus". Thanks, Shigeki Sakamoto.
On Tue, Jan 08, 2008 at 02:04:00PM +0900, S.Sakamoto wrote:
Hi
The libvirt on Xen-3.03 does not have the function that add/change/delete a Disk/NIC of inactive domain.
So, I made a patch which enabled add/change/delete of Disk/NIC of inactive domain. This patch will be able to change "disk" or "vif" parameter in configuration file with "attach-disk","attach-interface", "attach-device", "detach-disk", "detach-interface" and "detach-device" command, like "setmem" or "setvcpus".
The general principle of the patch is fine, but this change is not:
Index: src/conf.h =================================================================== RCS file: /data/cvs/libvirt/src/conf.h,v retrieving revision 1.4 diff -u -p -r1.4 conf.h --- src/conf.h 30 Nov 2007 15:43:42 -0000 1.4 +++ src/conf.h 8 Jan 2008 01:10:52 -0000 @@ -59,12 +59,31 @@ struct _virConfValue { };
/** + * virConfEntryPtr: + * used by configuration data + */ +typedef struct _virConfEntry virConfEntry; +typedef virConfEntry *virConfEntryPtr; + +struct _virConfEntry { + virConfEntryPtr next; + char* name; + char* comment; + virConfValuePtr value; +}; + +/** * virConfPtr: * a pointer to a parsed configuration file */ typedef struct _virConf virConf; typedef virConf *virConfPtr;
+struct _virConf { + const char* filename; + virConfEntryPtr entries; +}; + virConfPtr __virConfNew (void); virConfPtr __virConfReadFile (const char *filename); virConfPtr __virConfReadMem (const char *memory,
These struct definitions *intentionally* private. There are explicit APIs to access data within them. AFAICT, the 'virConfGetValue' API should be sufficient for the needs of your patch without accessing the struct data directly. 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 -=|
S.Sakamoto wrote:
These struct definitions *intentionally* private. Oops, I did not notice it...
I revised a patch not to access the struct data directly.
In principle this patch looks good. If no one else objects, I'll commit this, with a few of my own fixes (below). Rich. + (obj->stringval) && !strcmp((char*)obj->stringval, "hvm")) I'm going to use STREQ macro here instead of !strcmp. + if (ctxt) + xmlXPathFreeContext(ctxt); There are a few potential double-frees on the cleanup path. Need to set ctxt back to NULL after freeing it. -- 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
On Mon, Jan 14, 2008 at 12:25:44PM +0000, Richard W.M. Jones wrote:
S.Sakamoto wrote:
These struct definitions *intentionally* private. Oops, I did not notice it...
I revised a patch not to access the struct data directly.
In principle this patch looks good. If no one else objects, I'll commit this, with a few of my own fixes (below).
Yep, fine by me. 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 -=|
On Mon, Jan 14, 2008 at 01:51:56PM +0000, Daniel P. Berrange wrote:
On Mon, Jan 14, 2008 at 12:25:44PM +0000, Richard W.M. Jones wrote:
S.Sakamoto wrote:
These struct definitions *intentionally* private. Oops, I did not notice it...
I revised a patch not to access the struct data directly.
In principle this patch looks good. If no one else objects, I'll commit this, with a few of my own fixes (below).
Yep, fine by me.
Dunno yet if it was commited, in any case +1, 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/
These struct definitions *intentionally* private. Oops, I did not notice it...
I revised a patch not to access the struct data directly.
In principle this patch looks good. If no one else objects, I'll commit this, with a few of my own fixes (below).
Rich.
+ (obj->stringval) && !strcmp((char*)obj->stringval, "hvm"))
I'm going to use STREQ macro here instead of !strcmp.
+ if (ctxt) + xmlXPathFreeContext(ctxt);
There are a few potential double-frees on the cleanup path. Need to set ctxt back to NULL after freeing it.
Thank you for improving my patch! Because it looks like that no one else objects, please commit this. Thanks, Shigeki Sakamoto.
Resurrecting a two-week old thread... Was a version of this patch ever committed? I don't see it, but I may have missed it. I used an earlier version of the patch to fix a problem we've noticed, but I would like to see the final version of the patch. Thanks, Ryan S.Sakamoto wrote:
These struct definitions *intentionally* private. Oops, I did not notice it...
I revised a patch not to access the struct data directly. In principle this patch looks good. If no one else objects, I'll commit this, with a few of my own fixes (below).
Rich.
+ (obj->stringval) && !strcmp((char*)obj->stringval, "hvm"))
I'm going to use STREQ macro here instead of !strcmp.
+ if (ctxt) + xmlXPathFreeContext(ctxt);
There are a few potential double-frees on the cleanup path. Need to set ctxt back to NULL after freeing it.
Thank you for improving my patch! Because it looks like that no one else objects, please commit this.
Thanks, Shigeki Sakamoto.
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Feb 05, 2008 at 04:00:14PM -0800, Ryan Scott wrote:
Resurrecting a two-week old thread...
Was a version of this patch ever committed? I don't see it, but I may have missed it.
I used an earlier version of the patch to fix a problem we've noticed, but I would like to see the final version of the patch.
Well Apparently Rich didn't commit his version. So I applied the patch made the two changes he suggested, a bit of reformating, this should now be in CVS, thanks everybody ! Daniel
S.Sakamoto wrote:
These struct definitions *intentionally* private. Oops, I did not notice it...
I revised a patch not to access the struct data directly. In principle this patch looks good. If no one else objects, I'll commit this, with a few of my own fixes (below).
Rich.
+ (obj->stringval) && !strcmp((char*)obj->stringval, "hvm"))
I'm going to use STREQ macro here instead of !strcmp.
+ if (ctxt) + xmlXPathFreeContext(ctxt);
There are a few potential double-frees on the cleanup path. Need to set ctxt back to NULL after freeing it.
Thank you for improving my patch! Because it looks like that no one else objects, please commit this.
Thanks, Shigeki Sakamoto.
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- 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/
Daniel Veillard wrote:
On Tue, Feb 05, 2008 at 04:00:14PM -0800, Ryan Scott wrote:
Resurrecting a two-week old thread...
Was a version of this patch ever committed? I don't see it, but I may have missed it.
I used an earlier version of the patch to fix a problem we've noticed, but I would like to see the final version of the patch.
Well Apparently Rich didn't commit his version. So I applied the patch made the two changes he suggested, a bit of reformating,
this should now be in CVS,
Thanks for doing that. As you surmised, I didn't get around to it. 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
participants (5)
-
Daniel P. Berrange -
Daniel Veillard -
Richard W.M. Jones -
Ryan Scott -
S.Sakamoto