Re: [libvirt] qemudDomainSetMemory() / qemudDomainSetVcpus() error codes
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
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
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
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)
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
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)
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)
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
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
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)
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