Re: [PATCH 2/2] Allow memory to be specified in kvmctl

[Muli Ben-Yehuda <muli-7z/5BgaJwgfQT0dZR+AlfA@public.gmane.org>]

On Tue, Oct 09, 2007 at 04:31:54PM -0500, Anthony Liguori wrote:

> This patch adds a --memory option to kvmctl to allow the memory size of the
> guest to be specified.
> 
> Signed-off-by: Anthony Liguori <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> 
> diff --git a/user/main.c b/user/main.c
> index 9a57a24..7fc0924 100644
> --- a/user/main.c
> +++ b/user/main.c
> @@ -57,6 +57,7 @@ static __thread int vcpu;
>  static int apic_ipi_vector = 0xff;
>  static sigset_t kernel_sigmask;
>  static sigset_t ipi_sigmask;
> +static uint64_t memory_size = 128 * 1024 * 1024;
>  
>  struct vcpu_info {
>  	pid_t tid;
> @@ -147,7 +148,7 @@ static int test_inl(void *opaque, uint16_t addr, uint32_t *value)
>  
>  	switch (addr) {
>  	case 0xd1:
> -		*value = 128 * 1024 * 1024;
> +		*value = memory_size;
>  		break;
>  	default:
>  		printf("inl 0x%x\n", addr);
> @@ -328,14 +329,16 @@ static void start_vcpu(int n)
>  static void usage(const char *progname)
>  {
>  	fprintf(stderr,
> -		"Usage: %s [OPTIONS] [bootstrap] flatfile\n"
> -		"KVM test harness.\n"
> -		"\n"
> -		"  -s, --smp=NUM          create a VM with NUM virtual CPUs\n"
> -		"  -p, --protected-mode   start VM in protected mode\n"
> -		"  -h, --help             display this help screen and exit\n"
> -		"\n"
> -		"Report bugs to <kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>.\n"
> +"Usage: %s [OPTIONS] [bootstrap] flatfile\n"
> +"KVM test harness.\n"
> +"\n"
> +"  -s, --smp=NUM          create a VM with NUM virtual CPUs\n"
> +"  -p, --protected-mode   start VM in protected mode\n"
> +"  -m, --memory=NUM[GMKB] allocate NUM memory for virtual machine.  A suffix\n"
> +"                         can be used to change the unit (default: `M')\n"
> +"  -h, --help             display this help screen and exit\n"
> +"\n"
> +"Report bugs to <kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>.\n"
>  		, progname);
>  }
>  
> @@ -348,16 +351,18 @@ int main(int argc, char **argv)
>  {
>  	void *vm_mem;
>  	int i;
> -	const char *sopts = "s:ph";
> +	const char *sopts = "s:phm:";
>  	struct option lopts[] = {
>  		{ "smp", 1, 0, 's' },
>  		{ "protected-mode", 0, 0, 'p' },
> +		{ "memory", 1, 0, 'm' },
>  		{ "help", 0, 0, 'h' },
>  		{ 0 },
>  	};
>  	int opt_ind, ch;
>  	bool enter_protected_mode = false;
>  	int nb_args;
> +	char *endptr;
>  
>  	while ((ch = getopt_long(argc, argv, sopts, lopts, &opt_ind)) != -1) {
>  		switch (ch) {
> @@ -367,6 +372,24 @@ int main(int argc, char **argv)
>  		case 'p':
>  			enter_protected_mode = true;
>  			break;
> +		case 'm':
> +			memory_size = strtoull(optarg, &endptr, 0);
> +			switch (*endptr) {
> +			case 'G': case 'g':
> +				memory_size <<= 10;
> +			case '\0':
> +			case 'M': case 'm':
> +				memory_size <<= 10;
> +			case 'K': case 'k':
> +				memory_size <<= 10;
> +				break;

Cute trick with the fall-through and shifts... not quite Duff's
device, but cute. Please consider adding a /* fallthrough */ comment
to make it obvious.

> +			default:
> +				fprintf(stderr,
> +					"Unrecongized memory suffix: %c\n",
> +					*endptr);
> +				exit(1);
> +			}
> +			break;

How about adding a sanity check that memory_size makes sense here
rather than having kvm_create() fail obscurely? For example if the
user got the memory size wrong for some reason we'll end up with
memory_size = 0 here.

Cheers,
Muli
-- 
SYSTOR 2007 --- 1st Annual Haifa Systems and Storage Conference 2007
http://www.haifa.il.ibm.com/Workshops/systor2007/

Virtualization workshop: Oct 29th, 2007 | Storage workshop: Oct 30th, 2007

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/