Re: [libvirt] qemudDomainSetMemory() / qemudDomainSetVcpus() error codes

2008-06-06 Thread Daniel P. Berrange
On Thu, Jun 05, 2008 at 03:38:46PM -0700, Kaitlin Rupert wrote:
 Hello,
 
 I noticed qemudDomainSetMemory() / qemudDomainSetVcpus() use the 
 VIR_ERR_INTERNAL_ERROR error code when the caller attempts to set the 
 mem/vcpus of an active domain.
 
 In this case, returning something like VIR_ERR_NO_SUPPORT would indicate 
 that this is an invalid call - which would distinguish the error from a 
 failure that occurred during the call.

Yes, that would be a better choice in this scenario. Patches welcomed to
changed this :-)

Dan.
-- 
|: Red Hat, Engineering, London   -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 :|

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


Re: [libvirt] [PATCH] lxc: new tty for container

2008-06-06 Thread Daniel P. Berrange
On Thu, Jun 05, 2008 at 11:59:34PM -0700, Dave Leskovec wrote:
 The lxc driver xml parser treats the console element as input when it should 
 be
 output only.  This obviously causes problems when anything other than 
 /dev/ptmx
 is specified or when a container is shutdown and then restarted without
 redefining it (so that the console field is reset).  This patch treats the
 console element as output only and always creates a new device when starting a
 container.  Also fixed up a string overflow when storing the device name.

Yes, this is good. When using PTY's this is definitely output-only. No one
should be using the ancient manually allocated pre-Unix98 TTYs. I'd not come
across posix_openpt() before though - I'm used to openpty(). Since this is
a Linux specific driver though, there's no portability issues to worry about
so I'm happy with either.

Regards,
Daniel.
-- 
|: Red Hat, Engineering, London   -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 :|

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


Re: [libvirt] [PATCH] virDomainMemoryPeek - peek into guest memory

2008-06-06 Thread Daniel P. Berrange
On Thu, Jun 05, 2008 at 10:30:55PM +0100, Richard W.M. Jones wrote:
  
 +/* Memory peeking flags. */
 +typedef enum {
 +  VIR_MEMORY_VIRTUAL  = 1, /* addresses are virtual addresses */
 +} virDomainMemoryFlags;

Since there is only one flag, and it is compulsory I'm rather inclined
to say that virtual memory addressing should be the default with a flags
value of 0. Unless there is another mode, not yet implemented, that you 
think would be a better default in the future ?  Obviously keep the flags
arg for expansion regardless though.

 diff -u -p -r1.36 remote.c
 --- qemud/remote.c5 Jun 2008 21:12:26 -   1.36
 +++ qemud/remote.c5 Jun 2008 21:26:29 -
 @@ -938,6 +938,52 @@ remoteDispatchDomainBlockPeek (struct qe
  }
  
  static int
 +remoteDispatchDomainMemoryPeek (struct qemud_server *server ATTRIBUTE_UNUSED,
 +struct qemud_client *client,
 +remote_message_header *req,
 +remote_domain_memory_peek_args *args,
 +remote_domain_memory_peek_ret *ret)
 +{
 +virDomainPtr dom;
 +unsigned long long offset;
 +size_t size;
 +unsigned int flags;
 +CHECK_CONN (client);
 +
 +dom = get_nonnull_domain (client-conn, args-dom);
 +if (dom == NULL) {
 +remoteDispatchError (client, req, %s, _(domain not found));
 +return -2;
 +}
 +offset = args-offset;
 +size = args-size;
 +flags = args-flags;
 +
 +if (size  REMOTE_DOMAIN_BLOCK_PEEK_BUFFER_MAX) {
 +remoteDispatchError (client, req,
 + %s, _(size  maximum buffer size));
 +return -2;
 +}
 +
 +ret-buffer.buffer_len = size;
 +ret-buffer.buffer_val = malloc (size);
 +if (!ret-buffer.buffer_val) {
 +remoteDispatchError (client, req, %s, strerror (errno));
 +return -2;
 +}

The 'dom' object is leaking in the 2 error paths here  it'd be better
to use VIR_ALLOC_N(ret-buffer.buffer_val, size) here.

 diff -u -p -r1.14 remote_protocol.x
 --- qemud/remote_protocol.x   5 Jun 2008 21:12:27 -   1.14
 +++ qemud/remote_protocol.x   5 Jun 2008 21:26:29 -
 @@ -340,6 +340,17 @@ struct remote_domain_block_peek_ret {
  opaque bufferREMOTE_DOMAIN_BLOCK_PEEK_BUFFER_MAX;
  };
  
 +struct remote_domain_memory_peek_args {
 +remote_nonnull_domain dom;
 +unsigned hyper offset;
 +unsigned size;
 +unsigned flags;
 +};
 +
 +struct remote_domain_memory_peek_ret {
 +opaque bufferREMOTE_DOMAIN_BLOCK_PEEK_BUFFER_MAX;
 +};

Is it worth having a separate constant for the max memory size, independant
of the block peek size. 

 +/**
 + * virDomainMemoryPeek:
 + * @dom: pointer to the domain object
 + * @start: start of memory to peek
 + * @size: size of memory to peek
 + * @buffer: return buffer (must be at least size bytes)
 + * @flags: flags, see below
 + *
 + * This function allows you to read the contents of a domain's
 + * memory.
 + *
 + * The memory which is read is controlled by the 'start', 'size'
 + * and 'flags' parameters.
 + *
 + * If 'flags' is VIR_MEMORY_VIRTUAL then the 'start' and 'size'
 + * parameters are interpreted as virtual memory addresses for
 + * whichever task happens to be running on the domain at the
 + * moment.  Although this sounds haphazard it is in fact what
 + * you want in order to read Linux kernel state, because it
 + * ensures that pointers in the kernel image can be interpreted
 + * coherently.
 + *
 + * 'buffer' is the return buffer and must be at least 'size' bytes.
 + * 'size' may be 0 to test if the call would succeed.

Should we document and enforce a maximum value for size. The remote
driver at least has a maximum limit,, so perhaps we should enforce
that in the main API dispatcher here in virDomainMemoryPeek() impl

 +int
 +static int
 +qemudDomainMemoryPeek (virDomainPtr dom,
 +   unsigned long long offset, size_t size,
 +   void *buffer,
 +   unsigned int flags)
 +{
 +struct qemud_driver *driver = (struct qemud_driver 
 *)dom-conn-privateData;
 +struct qemud_vm *vm = qemudFindVMByID (driver, dom-id);
 +char cmd[256], *info;
 +char tmp[] = /tmp/qemumemXX;

It'd make the SELinux policy easier if we avoided the generic /tmp and put
the files in somewhere like /var/spool/libvirt/qemu/. Wouldn't even need to
use mkstemp() then - just have it named VMNAME.mem since it'd be in a safe
controlled directory

Regards,
Daniel.
-- 
|: Red Hat, Engineering, London   -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 :|

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


Re: [libvirt] PATCH: Switch all remaining code to memory alloc APIs

2008-06-06 Thread Daniel P. Berrange
On Mon, Jun 02, 2008 at 04:35:47PM +0200, Jim Meyering wrote:
 Daniel P. Berrange [EMAIL PROTECTED] wrote:
  This patch switches all remaining code over to use the memory allocation
  APIs, with exception of virsh which is going to be slightly more complex
 
  It was mostly a straight conversion - there were only a few places which 
  weren't checking for failure corecttly - the most notable being sexpr.c.

  @@ -266,7 +264,7 @@
   memset(zeros, 0, sizeof(zeros));
   
   /* XXX multiple pvs */
  -if ((vgargv = malloc(sizeof(char*) * (1))) == NULL) {
  +if (VIR_ALLOC_N(vgargv, 1)  0) {
   virStorageReportError(conn, VIR_ERR_NO_MEMORY, %s, _(command 
  line));
 
 That can be just
 
 if (VIR_ALLOC(vgargv)  0) {

I kept that as ALLOC_N to remind myself that this needs to change in the
future to support multiple PVs.

  @@ -172,15 +173,12 @@
   if (xenUnifiedNodeGetInfo(dom-conn, nodeinfo)  0)
   return(NULL);
   
  -cpulist = calloc(nb_cpu, sizeof(*cpulist));
  -if (cpulist == NULL)
  +if (VIR_ALLOC_N(cpulist, nb_cpu)  0)
   goto done;
  -cpuinfo = malloc(sizeof(*cpuinfo) * nb_vcpu);
  -if (cpuinfo == NULL)
  +if (VIR_ALLOC_N(cpuinfo, nb_vcpu)  0)
   goto done;
   cpumaplen = VIR_CPU_MAPLEN(VIR_NODEINFO_MAXCPUS(nodeinfo));
  -cpumap = (unsigned char *) calloc(nb_vcpu, cpumaplen);
  -if (cpumap == NULL)
  +if (VIR_ALLOC_N(cpumap, nb_vcpu * cpumaplen)  0)
   goto done;
 
 At first I thought it didn't matter that the product wasn't
 checked for overflow, but then I spent a couple minutes trying
 to find if/where nb_vcpu was guaranteed to be small enough
 that we don't have to worry.  There may well be code to ensure
 that, but if so, it's too far from this point of use for my taste,
 so I think it's best to add an explicit overflow check here, i.e.,
 
if (xalloc_oversized(nb_vcpu, cpumaplen) ||
VIR_ALLOC_N(cpumap, nb_vcpu * cpumaplen)  0)
goto done;

Yep, this does really need checking

Dan.
-- 
|: Red Hat, Engineering, London   -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 :|

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


[libvirt] Question about compilation on MinGW(StorageAPI)

2008-06-06 Thread Atsushi SAKAI
Hi, Rich and Dan

I have a question about libvirt on MinGW.
As you know, MinGW supports remote management only.
In this situation, Storage API(dir) should be off.
Can I configure it? or other way exists?


One more question on MinGW with Ruby,
Does someone already test the Ruby bindings on MinGW?
I am stacked this on configuration of rake on MinGW.

Thanks
Atsushi SAKAI



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


Re: [libvirt] [PATCH] virDomainMemoryPeek - peek into guest memory

2008-06-06 Thread Richard W.M. Jones
On Fri, Jun 06, 2008 at 11:25:41AM +0100, Daniel P. Berrange wrote:
 On Thu, Jun 05, 2008 at 10:30:55PM +0100, Richard W.M. Jones wrote:
   
  +/* Memory peeking flags. */
  +typedef enum {
  +  VIR_MEMORY_VIRTUAL  = 1, /* addresses are virtual addresses 
  */
  +} virDomainMemoryFlags;
 
 Since there is only one flag, and it is compulsory I'm rather inclined
 to say that virtual memory addressing should be the default with a flags
 value of 0. Unless there is another mode, not yet implemented, that you 
 think would be a better default in the future ?  Obviously keep the flags
 arg for expansion regardless though.

The reason I wanted this flag is because I think this behaviour is
unexpected, so it's worth remarking on it.  I would (naively) have
expected a memory-peek call to peek physical memory, even though that
isn't very useful behaviour if you want to actually analyze the
memory.  So the flag makes sure people realize that the peeking does a
virtual to physical address mapping.  I fully expect that we would add
a VIR_MEMORY_PHYSICAL flag at some point.

Thanks for looking at the rest of the patch.  I'll make an updated
version soon with those things fixed.

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/

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


Re: [libvirt] Question about compilation on MinGW(StorageAPI)

2008-06-06 Thread Daniel P. Berrange
On Fri, Jun 06, 2008 at 07:34:32PM +0900, Atsushi SAKAI wrote:
 Hi, Rich and Dan
 
 I have a question about libvirt on MinGW.
 As you know, MinGW supports remote management only.
 In this situation, Storage API(dir) should be off.
 Can I configure it? or other way exists?

The storage APIs will only be built if the daemon is enabled. So you can
turn it off with the configure flag:

--without-libvirtd

There are also flags to turn on/off individual storage backend drivers,
but they should also automatically turn themselves off if they detect
missing functionality.

Regards,
Daniel.
-- 
|: Red Hat, Engineering, London   -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 :|

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


Re: [libvirt] Question about compilation on MinGW(StorageAPI)

2008-06-06 Thread Atsushi SAKAI
Hi, Dan

Thank you, I remenber the Mail from Rich.
http://www.redhat.com/archives/libvir-list/2008-January/msg00017.html
virsh seems working again.

Current problem on MinGW are 2points(These are avoidable on manually).

1) siginfo_t should comment out.(it effects driver.h/internal.h/libvirt.c on 
src directory)
2) Cannot compile tests directory

Thanks
Atsushi SAKAI



Daniel P. Berrange [EMAIL PROTECTED] wrote:

 On Fri, Jun 06, 2008 at 07:34:32PM +0900, Atsushi SAKAI wrote:
  Hi, Rich and Dan
  
  I have a question about libvirt on MinGW.
  As you know, MinGW supports remote management only.
  In this situation, Storage API(dir) should be off.
  Can I configure it? or other way exists?
 
 The storage APIs will only be built if the daemon is enabled. So you can
 turn it off with the configure flag:
 
 --without-libvirtd
 
 There are also flags to turn on/off individual storage backend drivers,
 but they should also automatically turn themselves off if they detect
 missing functionality.
 
 Regards,
 Daniel.
 -- 
 |: Red Hat, Engineering, London   -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 :|


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


[libvirt] PATCH: Fix check for inactive network

2008-06-06 Thread Daniel P. Berrange
When building up the QEMU command line args we forgot to check for a NULL
when creating the TAP device arg. This can happen if the network being
connected to the VM is not running. This is something I broke in the
refactoring of the ARGV construction in the QEMU driver.

Dan.

Index: src/qemu_conf.c
===
RCS file: /data/cvs/libvirt/src/qemu_conf.c,v
retrieving revision 1.76
diff -u -p -r1.76 qemu_conf.c
--- src/qemu_conf.c 29 May 2008 19:20:23 -  1.76
+++ src/qemu_conf.c 6 Jun 2008 13:29:17 -
@@ -2679,8 +2679,13 @@ int qemudBuildCommandLine(virConnectPtr 
 switch (net-type) {
 case QEMUD_NET_NETWORK:
 case QEMUD_NET_BRIDGE:
-ADD_ARG(qemudNetworkIfaceConnect(conn, driver, vm, net, vlan));
-break;
+{
+char *tap = qemudNetworkIfaceConnect(conn, driver, vm, 
net, vlan);
+if (tap == NULL)
+goto error;
+ADD_ARG(tap);
+break;
+}
 
 case QEMUD_NET_ETHERNET:
 {


-- 
|: Red Hat, Engineering, London   -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 :|

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


Re: [libvirt] PATCH: Fix check for inactive network

2008-06-06 Thread Richard W.M. Jones
On Fri, Jun 06, 2008 at 02:31:10PM +0100, Daniel P. Berrange wrote:
 When building up the QEMU command line args we forgot to check for a NULL
 when creating the TAP device arg. This can happen if the network being
 connected to the VM is not running. This is something I broke in the
 refactoring of the ARGV construction in the QEMU driver.

+1 -- I just got bitten by this bug.

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

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


Re: [libvirt] Question about compilation on MinGW(StorageAPI)

2008-06-06 Thread Daniel P. Berrange
On Fri, Jun 06, 2008 at 10:03:41PM +0900, Atsushi SAKAI wrote:
 Hi, Dan
 
 Thank you, I remenber the Mail from Rich.
 http://www.redhat.com/archives/libvir-list/2008-January/msg00017.html
 virsh seems working again.
 
 Current problem on MinGW are 2points(These are avoidable on manually).
 
 1) siginfo_t should comment out.(it effects driver.h/internal.h/libvirt.c on 
 src directory)

Hmm, all the APIs with  virState in their name should be disabled
by a #ifdef  WITH_LIBVIRTD, since they only make sense in the context
of the daemon.

 2) Cannot compile tests directory

Can you post the compile errors for this - it ought to be easy enough to
fix

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -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 :|

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