Re: [Libvir] [PATCH] increase storage domains of a definition file
Hi, Dan
Thank you for comment.
Shigeki does not repond this issue at this moment.
I will comment on this instead.
He should respond this issue quickly!
In his survey,
Many area is allocated as constant data(usually about 1KB size)
in libvirt.
He plan to fix this issue(dynamical allocation(which you suggested)
as next stage (Fedora 8 or later.)
As a Fedora 7 phase solution,
He choses constant data size for solving this issue.
I also think 130KB is too big.
Thanks
Atsushi SAKAI
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Fri, Apr 20, 2007 at 02:43:31PM +0900, S.Sakamoto wrote:
> Hi,
>
> When I install by virt-intall with long command
> (e.g."# /usr/sbin/virt-install --name testvm --ram 350 --vcpus 2 --file
/root/test.img --file-size 1 --file /root/tmp02.img --
> file-size 1 --file /root/tmp03.img --file-size 1 --file /root/tmp04.img --file-size
1 --file /root/tmp05.img --file-size
> 1 --file /root/tmp06.img --file-size 1 --file /root/tmp07.img --file-size 1 --file
/root/tmp08.img --file-size 1 --file
> /root/tmp09.img --file-size 1 --file /root/tmp10.img --file-size 1 --file
/root/tmp11.img --file-size 1 --file
> /root/tmp12.img --file-size 1 --file /root/tmp13.img --file-size 1 --file
/root/tmp14.img --file-size 1 --file
> /root/tmp15.img --file-size 1 --file /root/tmp16.img --file-size 1 --vnc --paravirt
--location ftp://xx.xx.xx.xx/rhel5ga_x86 --noautoconsole --
> debug"),
> put out "abort".
> Because a definition file is long, it overflows from buffer.
>
> So, this patch increase buffer size.
>
>
> Thanks,
> Shigeki Sakamoto.
>
>
> Index: src/internal.h
> ===================================================================
> RCS file: /data/cvs/libvirt/src/internal.h,v
> retrieving revision 1.37
> diff -u -p -r1.37 internal.h
> --- src/internal.h 4 Apr 2007 14:19:49 -0000 1.37
> +++ src/internal.h 19 Apr 2007 11:01:12 -0000
> @@ -106,6 +106,11 @@ extern "C" {
> #define VIR_CONNECT_RO 1
>
> /**
> + * buffer size for definition file
> + */
> +#define VIR_XML_STRING_BUFLEN (1024 + PATH_MAX * 16 + FILENAME_MAX * 16)
Considering we were only rarely overflowing the 1024 byte buffer,
I think increasing the default buffer size to 132096 - is little bit
of overkill. I'd rather see all buffers dynamically allocated and
then this constant wouldn't be needed.
> +
> +/**
> * _virConnect:
> *
> * Internal structure associated to a connection
> Index: src/xend_internal.c
> ===================================================================
> RCS file: /data/cvs/libvirt/src/xend_internal.c,v
> retrieving revision 1.109
> diff -u -p -r1.109 xend_internal.c
> --- src/xend_internal.c 13 Apr 2007 14:08:38 -0000 1.109
> +++ src/xend_internal.c 19 Apr 2007 11:01:16 -0000
> @@ -587,7 +587,7 @@ static int
> xend_op_ext2(virConnectPtr xend, const char *path, char *error,
> size_t n_error, const char *key, va_list ap)
> {
> - char ops[1024];
> + char ops[VIR_XML_STRING_BUFLEN];
This gives a 130 KB static string. I can't help thinking this method
would be better off using the dyn allocated virBuffer* routines instead
of a static string & snprintf.
NB, this patch doesn't modify the proxy code at all which has a 4096
byte field defined for this, on the wire.
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 -=|
--
Libvir-list mailing list
Libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list