From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Daniel P. Berrange" <berrange@redhat.com>
Subject: Re: [libvirt] [PATCH 7/7] kvmtool: Implementation for kvm tool driver
Date: Tue, 6 Dec 2011 14:55:38 +0000
Message-ID: <20111206145538.GG7937@redhat.com>
References: <1321012626-31713-1-git-send-email-jyang@redhat.com>
 <1321012626-31713-9-git-send-email-jyang@redhat.com>
Reply-To: "Daniel P. Berrange" <berrange@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Cc: libvir-list@redhat.com, kvm@vger.kernel.org,
	penberg@cs.helsinki.fi, levinsasha928@gmail.com,
	gorcunov@gmail.com, mingo@elte.hu, asias.hejun@gmail.com
To: Osier Yang <jyang@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:19679 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S933395Ab1LFO41 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 6 Dec 2011 09:56:27 -0500
Content-Disposition: inline
In-Reply-To: <1321012626-31713-9-git-send-email-jyang@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Fri, Nov 11, 2011 at 07:57:06PM +0800, Osier Yang wrote:
> Basically, the drivers is implemented by using kvm tool binary
> currently, (see ./kvm help for more info).
> 
> Current implementation supports define/undefine, start/destroy/,
> suspend/resume, connect to guest console via "virsh console",
> and balloon memory with with "virsh setmem" (using ./kvm balloon
> command). Also as it supports cgroup controllers "cpuacct", and
> "memory", so some other commands like "schedinfo", "memtune" can
> also work. Some other commands such as "domid", "domname", "dumpxml"
> ,"autostart", etc. are supported, as the driver is designed
> as a "stateful" driver, those APIs just need to talk with libvirtd
> simply.
> 
> As Native Linux KVM Tool is designed for both non-root and root users,
> the driver is designed just like QEMU, supports two modes of the
> connection:
> 
>     kvmtool:///system
>     kvmtool+unix:///system
> 
>     kvmtool:///session
>     kvmtool+unix:///session
> 
> An example of the domain XML (all the XMLs supported currently are
> listed):
> 
> % virsh -c kvm:///system dumpxml kvm_test
> <domain type='kvmtool'>
>   <name>kvm_test</name>
>   <uuid>88bf38f1-b6ab-cfa6-ab53-4b4c0993d894</uuid>
>   <memory>524288</memory>
>   <currentMemory>524288</currentMemory>
>   <vcpu>1</vcpu>
>   <os>
>     <type arch='x86_64'>hvm</type>
>     <kernel>/boot/bzImage</kernel>
>     <boot dev='hd'/>
>   </os>
>   <clock offset='utc'/>
>   <on_poweroff>destroy</on_poweroff>
>   <on_reboot>restart</on_reboot>
>   <on_crash>restart</on_crash>
>   <devices>
>     <emulator>/usr/bin/kvmtool</emulator>
>     <disk type='file' device='disk'>
>       <source file='/var/lib/libvirt/images/linux-0.2.img'/>
>       <target dev='vda' bus='virtio'/>
>     </disk>
>     <filesystem type='mount' accessmode='passthrough'>
>       <source dir='/tmp'/>
>       <target dir='/mnt'/>
>     </filesystem>
>     <console type='pty'>
>       <target type='serial' port='0'/>
>     </console>
>     <memballoon model='virtio'/>
>   </devices>
> </domain>
> ---
>  cfg.mk                       |    1 +
>  daemon/Makefile.am           |    4 +
>  daemon/libvirtd.c            |    7 +
>  po/POTFILES.in               |    2 +
>  src/Makefile.am              |   36 +-
>  src/kvmtool/kvmtool_conf.c   |  130 ++
>  src/kvmtool/kvmtool_conf.h   |   66 +
>  src/kvmtool/kvmtool_driver.c | 3079 ++++++++++++++++++++++++++++++++++++++++++
>  src/kvmtool/kvmtool_driver.h |   29 +

My main suggestion here would be to split up the kvmtool_driver.c
file into 3 parts as we did with the QEMU driver.

  kvmtool_driver.c   -> Basic libvirt API glue
  kvmtool_command.c  -> ARGV generation
  kvmtool_process.c  -> KVMtool process start/stop/autostart/autodestroy

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|