From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=53043 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PgLeL-0005bL-Tl for qemu-devel@nongnu.org; Fri, 21 Jan 2011 13:23:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PgLeK-0000hc-JH for qemu-devel@nongnu.org; Fri, 21 Jan 2011 13:23:53 -0500 Received: from e4.ny.us.ibm.com ([32.97.182.144]:39212) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PgLeK-0000hX-Fa for qemu-devel@nongnu.org; Fri, 21 Jan 2011 13:23:52 -0500 Received: from d01dlp01.pok.ibm.com (d01dlp01.pok.ibm.com [9.56.224.56]) by e4.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p0LI3Qnf004080 for ; Fri, 21 Jan 2011 13:05:44 -0500 Received: from d01relay04.pok.ibm.com (d01relay04.pok.ibm.com [9.56.227.236]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id BF0AE728063 for ; Fri, 21 Jan 2011 13:23:43 -0500 (EST) Received: from d03av06.boulder.ibm.com (d03av06.boulder.ibm.com [9.17.195.245]) by d01relay04.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p0LINhVB179788 for ; Fri, 21 Jan 2011 13:23:43 -0500 Received: from d03av06.boulder.ibm.com (loopback [127.0.0.1]) by d03av06.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p0LISDZb027658 for ; Fri, 21 Jan 2011 11:28:13 -0700 Message-ID: <4D39CF2C.1000407@linux.vnet.ibm.com> Date: Fri, 21 Jan 2011 12:23:40 -0600 From: Michael Roth MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [RFC][PATCH v6 08/23] virtagent: add va.getfile RPC References: <1295270117-24760-1-git-send-email-mdroth@linux.vnet.ibm.com> <1295270117-24760-9-git-send-email-mdroth@linux.vnet.ibm.com> <4D39B716.1010400@redhat.com> <20110121172026.GK12589@redhat.com> In-Reply-To: <20110121172026.GK12589@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: agl@linux.vnet.ibm.com, stefanha@linux.vnet.ibm.com, Jes Sorensen , marcel.mittelstaedt@de.ibm.com, qemu-devel@nongnu.org, markus_mueller@de.ibm.com, aliguori@linux.vnet.ibm.com, ryanh@us.ibm.com, abeekhof@redhat.com On 01/21/2011 11:20 AM, Daniel P. Berrange wrote: > On Fri, Jan 21, 2011 at 05:40:54PM +0100, Jes Sorensen wrote: >> On 01/17/11 14:15, Michael Roth wrote: >>> Add RPC to retrieve a guest file. This interface is intended >>> for smaller reads like peeking at logs and /proc and such. >>> >>> Signed-off-by: Michael Roth >>> --- >>> virtagent-server.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> 1 files changed, 59 insertions(+), 0 deletions(-) >>> >>> diff --git a/virtagent-server.c b/virtagent-server.c >>> index c38a9e0..af4b940 100644 >>> --- a/virtagent-server.c >>> +++ b/virtagent-server.c >>> @@ -62,12 +62,71 @@ out: >>> return ret; >>> } >>> >>> +/* RPC functions common to guest/host daemons */ >>> + >>> +/* va_getfile(): return file contents >>> + * rpc return values: >>> + * - base64-encoded file contents >>> + */ >>> +static xmlrpc_value *va_getfile(xmlrpc_env *env, >>> + xmlrpc_value *params, >>> + void *user_data) >>> +{ >>> + const char *path; >>> + char *file_contents = NULL; >>> + char buf[VA_FILEBUF_LEN]; >> >> malloc()! >> >>> + int fd, ret, count = 0; >>> + xmlrpc_value *result = NULL; >>> + >>> + /* parse argument array */ >>> + xmlrpc_decompose_value(env, params, "(s)",&path); >>> + if (env->fault_occurred) { >>> + return NULL; >>> + } >>> + >>> + SLOG("va_getfile(), path:%s", path); >>> + >>> + fd = open(path, O_RDONLY); >>> + if (fd == -1) { >>> + LOG("open failed: %s", strerror(errno)); >>> + xmlrpc_faultf(env, "open failed: %s", strerror(errno)); >>> + return NULL; >>> + } >>> + >>> + while ((ret = read(fd, buf, VA_FILEBUF_LEN))> 0) { >>> + file_contents = qemu_realloc(file_contents, count + VA_FILEBUF_LEN); >>> + memcpy(file_contents + count, buf, ret); >> >> Sorry, I brought this up before. This realloc() stuff is a disaster >> waiting to happen. Please remove it from the patch series, until you >> have an implementation that copies over a page of the time. > > I can understand the need of virtagent for lifecycle control/interactions > with the guest OS (reboot, shutdown, ping, screen lock/unlock, etc), but > do we really want to reinvent libguestfs for file access ? A little dev > work could enable users to install the libguestfs agent into a guest OS, > and access it from the host over virtio-serial + the libguestfs API. File/dmesg/etc access is a bit of a grey area. Technically it's not lifecycle-specific, but it tends to become a requirement for higher-level management policies, and being reliant on external tools to provide what, at least in our case, has been an extremely common request/requirement, greatly reduces the usefulness of such an agent. Ultimately however these interfaces would be exposed via libvirt, which libguestfs already makes use of, so it'd be a logically way to extend it for disk access to live guests. getfile() is confusingly named however, it's really just a means to peek at a text file like /proc/meminfo. general file access will be done via a stateful interface that implements similar semantics to open()/read()/write()/close(). > > This would be quite compelling usage model for app developers, because > it would mean whether the guest OS was running, or shutoff, they can > use the same libguestfs API for processing guest filesystem images. > The level of functionality provided by libguestfs is really quite > considerable now, letting you do pretty much any operation against > files that you could do via local POSIX for non-virt access, as > well as providing many useful higher level constructs > > Regards, > Daniel