Re: [Libvir] [PATCH] change a Disk/Nic of inactive domain

2008-02-06 Thread Daniel Veillard
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/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] [PATCH] change a Disk/Nic of inactive domain

2008-02-06 Thread Richard W.M. Jones

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


smime.p7s
Description: S/MIME Cryptographic Signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] [PATCH] change a Disk/Nic of inactive domain

2008-02-05 Thread Ryan Scott


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


--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] [PATCH] change a Disk/Nic of inactive domain

2008-01-21 Thread S.Sakamoto
  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


Re: [Libvir] [PATCH] change a Disk/Nic of inactive domain

2008-01-19 Thread Daniel Veillard
On Mon, Jan 14, 2008 at 01:51:56PM +, Daniel P. Berrange wrote:
 On Mon, Jan 14, 2008 at 12:25:44PM +, 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/
[EMAIL PROTECTED]  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] [PATCH] change a Disk/Nic of inactive domain

2008-01-14 Thread Richard W.M. Jones

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


smime.p7s
Description: S/MIME Cryptographic Signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] [PATCH] change a Disk/Nic of inactive domain

2008-01-14 Thread Daniel P. Berrange
On Mon, Jan 14, 2008 at 12:25:44PM +, 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  -=| 

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [Libvir] [PATCH] change a Disk/Nic of inactive domain

2008-01-08 Thread Daniel P. Berrange
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 -  1.4
 +++ src/conf.h  8 Jan 2008 01:10:52 -
 @@ -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  -=| 

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list