[PATCH] libxenlight: implement libxl_set_memory_target

[PATCH] libxenlight: implement libxl_set_memory_target

[Stefano Stabellini]

Hi all,
this patch adds a target_memkb parameter to libxl_domain_build_info to
set the target memory for the VM at build time and a new function
called libxl_set_memory_target to dynamically modify the memory target
of a VM at run time.
Finally a new command "mem-set" is added to xl that calls directly
libxl_set_memory_target.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

---

diff -r fe3c8ba27d2f tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Tue Dec 08 17:02:15 2009 +0000
+++ b/tools/libxl/libxl.c	Tue Dec 08 17:14:35 2009 +0000
@@ -935,6 +935,7 @@
     memset(&b_info, 0x00, sizeof(libxl_domain_build_info));
     b_info.max_vcpus = 1;
     b_info.max_memkb = 32 * 1024;
+    b_info.target_memkb = b_info.max_memkb;
     b_info.kernel = "/usr/lib/xen/boot/ioemu-stubdom.gz";
     b_info.u.pv.cmdline = libxl_sprintf(ctx, " -d %d", info->domid);
     b_info.u.pv.ramdisk = "";
@@ -2264,6 +2265,7 @@
     b_info->vpt_align = -1;
     b_info->max_vcpus = 1;
     b_info->max_memkb = 32 * 1024;
+    b_info->target_memkb = b_info->max_memkb;
     if (c_info->hvm) {
         b_info->shadow_memkb = libxl_get_required_shadow_memory(b_info->max_memkb, b_info->max_vcpus);
         b_info->video_memkb = 8 * 1024;
@@ -2356,4 +2358,22 @@
         console->build_state = state;
 }
 
+int libxl_set_memory_target(struct libxl_ctx *ctx, uint32_t domid, uint32_t target_memkb)
+{
+    int rc = 0;
+    uint32_t videoram;
+    char *videoram_s = NULL;
+    char *dompath = libxl_xs_get_dompath(ctx, domid);
 
+    videoram_s = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/memory/videoram", dompath));
+    if (!videoram_s)
+        return -1;
+    videoram = atoi(videoram_s);
+
+    libxl_xs_write(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/memory/target", dompath), "%lu", target_memkb);
+    if ((rc = xc_domain_setmaxmem(ctx->xch, domid, target_memkb + LIBXL_MAXMEM_CONSTANT)) != 0) return rc;
+    rc = xc_domain_memory_set_pod_target(ctx->xch, domid, (target_memkb - videoram) / 4, NULL, NULL, NULL);
+    return rc;
+}
+
+
diff -r fe3c8ba27d2f tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Tue Dec 08 17:02:15 2009 +0000
+++ b/tools/libxl/libxl.h	Tue Dec 08 17:14:35 2009 +0000
@@ -66,6 +66,7 @@
     int max_vcpus;
     int cur_vcpus;
     uint32_t max_memkb;
+    uint32_t target_memkb;
     uint32_t video_memkb;
     uint32_t shadow_memkb;
     const char *kernel;
@@ -305,6 +306,8 @@
 int libxl_domain_pause(struct libxl_ctx *ctx, uint32_t domid);
 int libxl_domain_unpause(struct libxl_ctx *ctx, uint32_t domid);
 
+int libxl_set_memory_target(struct libxl_ctx *ctx, uint32_t domid, uint32_t target_memkb);
+
 int libxl_console_attach(struct libxl_ctx *ctx, uint32_t domid, int cons_num);
 
 struct libxl_dominfo * libxl_domain_list(struct libxl_ctx *ctx, int *nb_domain);
diff -r fe3c8ba27d2f tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Tue Dec 08 17:02:15 2009 +0000
+++ b/tools/libxl/libxl_dom.c	Tue Dec 08 17:14:35 2009 +0000
@@ -52,7 +52,7 @@
     if (info->vpt_align != -1)
         xc_set_hvm_param(ctx->xch, domid, HVM_PARAM_VPT_ALIGN, (unsigned long) info->vpt_align);
     xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus);
-    xc_domain_setmaxmem(ctx->xch, domid, info->max_memkb + info->video_memkb);
+    xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + LIBXL_MAXMEM_CONSTANT);
     xc_domain_set_memmap_limit(ctx->xch, domid, 
             (info->hvm) ? info->max_memkb : 
             (info->max_memkb + info->u.pv.slack_memkb));
@@ -81,7 +81,9 @@
     ents[0] = "memory/static-max";
     ents[1] = libxl_sprintf(ctx, "%d", info->max_memkb);
     ents[2] = "memory/target";
-    ents[3] = libxl_sprintf(ctx, "%d", info->max_memkb); /* PROBABLY WRONG */
+    ents[3] = libxl_sprintf(ctx, "%d", info->target_memkb);
+    ents[2] = "memory/videoram";
+    ents[3] = libxl_sprintf(ctx, "%d", info->video_memkb);
     ents[4] = "domid";
     ents[5] = libxl_sprintf(ctx, "%d", domid);
     ents[6] = "store/port";
@@ -145,7 +147,7 @@
 {
     int ret;
 
-    ret = xc_hvm_build(ctx->xch, domid, info->max_memkb / 1024, info->kernel);
+    ret = xc_hvm_build_target_mem(ctx->xch, domid, (info->max_memkb - info->video_memkb) / 1024, (info->target_memkb - info->video_memkb) / 1024, info->kernel);
     if (ret) {
         XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, ret, "hvm building failed");
         return ERROR_FAIL;
diff -r fe3c8ba27d2f tools/libxl/libxl_internal.h
--- a/tools/libxl/libxl_internal.h	Tue Dec 08 17:02:15 2009 +0000
+++ b/tools/libxl/libxl_internal.h	Tue Dec 08 17:14:35 2009 +0000
@@ -31,6 +31,7 @@
 #define LIBXL_DEVICE_MODEL_START_TIMEOUT 10
 #define LIBXL_XENCONSOLE_LIMIT 1048576
 #define LIBXL_XENCONSOLE_PROTOCOL "vt100"
+#define LIBXL_MAXMEM_CONSTANT (1 * 1024 * 1024)
 #define QEMU_SIGNATURE "QemuDeviceModelRecord"
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
diff -r fe3c8ba27d2f tools/libxl/xl.c
--- a/tools/libxl/xl.c	Tue Dec 08 17:02:15 2009 +0000
+++ b/tools/libxl/xl.c	Tue Dec 08 17:14:35 2009 +0000
@@ -85,6 +85,7 @@
     printf("vpt_align: %d\n", b_info->vpt_align);
     printf("max_vcpus: %d\n", b_info->max_vcpus);
     printf("max_memkb: %d\n", b_info->max_memkb);
+    printf("target_memkb: %d\n", b_info->target_memkb);
     printf("kernel: %s\n", b_info->kernel);
     printf("hvm: %d\n", b_info->hvm);
 
@@ -283,8 +284,10 @@
     if (config_lookup_int (&config, "vcpus", &l) == CONFIG_TRUE)
         b_info->max_vcpus = l;
 
-    if (config_lookup_int (&config, "memory", &l) == CONFIG_TRUE)
+    if (config_lookup_int (&config, "memory", &l) == CONFIG_TRUE) {
         b_info->max_memkb = l * 1024;
+        b_info->target_memkb = b_info->max_memkb;
+    }
 
     if (config_lookup_int (&config, "shadow_memory", &l) == CONFIG_TRUE)
         b_info->shadow_memkb = l * 1024;
@@ -756,6 +759,7 @@
         printf(" restore                       restore a domain from a saved state\n\n");
         printf(" cd-insert                     insert a cdrom into a guest's cd drive\n\n");
         printf(" cd-eject                      eject a cdrom from a guest's cd drive\n\n");
+        printf(" mem-set                       set the current memory usage for a domain\n\n");
     } else if(!strcmp(command, "create")) {
         printf("Usage: xl create <ConfigFile> [options] [vars]\n\n");
         printf("Create a domain based on <ConfigFile>.\n\n");
@@ -804,7 +808,52 @@
     } else if (!strcmp(command, "cd-eject")) {
         printf("Usage: xl cd-eject <Domain> <VirtualDevice>\n\n");
         printf("Eject a cdrom from a guest's cd drive.\n\n");
+    } else if (!strcmp(command, "mem-set")) {
+        printf("Usage: xl mem-set <Domain> <MemKB>\n\n");
+        printf("Set the current memory usage for a domain.\n\n");
     }
+}
+
+void set_memory_target(char *p, char *mem)
+{
+    struct libxl_ctx ctx;
+    uint32_t domid;
+
+    libxl_ctx_init(&ctx);
+    libxl_ctx_set_log(&ctx, log_callback, NULL);
+
+    if (libxl_param_to_domid(&ctx, p, &domid) < 0) {
+        fprintf(stderr, "%s is an invalid domain identifier\n", p);
+        exit(2);
+    }
+    libxl_set_memory_target(&ctx, domid, atoi(mem));
+}
+
+int main_memset(int argc, char **argv)
+{
+    int opt = 0;
+    char *p = NULL, *mem;
+
+    while ((opt = getopt(argc, argv, "h:")) != -1) {
+        switch (opt) {
+        case 'h':
+            help("mem-set");
+            exit(0);
+        default:
+            fprintf(stderr, "option not supported\n");
+            break;
+        }
+    }
+    if (optind >= argc - 1) {
+        help("mem-set");
+        exit(2);
+    }
+
+    p = argv[optind];
+    mem = argv[optind + 1];
+
+    set_memory_target(p, mem);
+    exit(0);
 }
 
 void console(char *p, int cons_num)
@@ -1436,6 +1485,8 @@
         main_cd_insert(argc - 1, argv + 1);
     } else if (!strcmp(argv[1], "cd-eject")) {
         main_cd_eject(argc - 1, argv + 1);
+    } else if (!strcmp(argv[1], "mem-set")) {
+        main_memset(argc - 1, argv + 1);
     } else if (!strcmp(argv[1], "help")) {
         if (argc > 2)
             help(argv[2]);

Re: [PATCH] libxenlight: implement libxl_set_memory_target

[Andres Lagar-Cavilla]

Hi,
couple of comments:
- PV domains without videoram won't be able to use this
- Further, doesn't the PV domain build function need to use 
target_memkb? (That's my read of what xend does at least)
- Finally, LIBXL_MAXMEM_CONSTANT looks like an "evil constant we should 
avoid". Where did it come from?
Andres
> Hi all,
> this patch adds a target_memkb parameter to libxl_domain_build_info to
> set the target memory for the VM at build time and a new function
> called libxl_set_memory_target to dynamically modify the memory target
> of a VM at run time.
> Finally a new command "mem-set" is added to xl that calls directly
> libxl_set_memory_target.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>
> ---
>
> diff -r fe3c8ba27d2f tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c	Tue Dec 08 17:02:15 2009 +0000
> +++ b/tools/libxl/libxl.c	Tue Dec 08 17:14:35 2009 +0000
> @@ -935,6 +935,7 @@
>      memset(&b_info, 0x00, sizeof(libxl_domain_build_info));
>      b_info.max_vcpus = 1;
>      b_info.max_memkb = 32 * 1024;
> +    b_info.target_memkb = b_info.max_memkb;
>      b_info.kernel = "/usr/lib/xen/boot/ioemu-stubdom.gz";
>      b_info.u.pv.cmdline = libxl_sprintf(ctx, " -d %d", info->domid);
>      b_info.u.pv.ramdisk = "";
> @@ -2264,6 +2265,7 @@
>      b_info->vpt_align = -1;
>      b_info->max_vcpus = 1;
>      b_info->max_memkb = 32 * 1024;
> +    b_info->target_memkb = b_info->max_memkb;
>      if (c_info->hvm) {
>          b_info->shadow_memkb = libxl_get_required_shadow_memory(b_info->max_memkb, b_info->max_vcpus);
>          b_info->video_memkb = 8 * 1024;
> @@ -2356,4 +2358,22 @@
>          console->build_state = state;
>  }
>  
> +int libxl_set_memory_target(struct libxl_ctx *ctx, uint32_t domid, uint32_t target_memkb)
> +{
> +    int rc = 0;
> +    uint32_t videoram;
> +    char *videoram_s = NULL;
> +    char *dompath = libxl_xs_get_dompath(ctx, domid);
>  
> +    videoram_s = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/memory/videoram", dompath));
> +    if (!videoram_s)
> +        return -1;
> +    videoram = atoi(videoram_s);
> +
> +    libxl_xs_write(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/memory/target", dompath), "%lu", target_memkb);
> +    if ((rc = xc_domain_setmaxmem(ctx->xch, domid, target_memkb + LIBXL_MAXMEM_CONSTANT)) != 0) return rc;
> +    rc = xc_domain_memory_set_pod_target(ctx->xch, domid, (target_memkb - videoram) / 4, NULL, NULL, NULL);
> +    return rc;
> +}
> +
> +
> diff -r fe3c8ba27d2f tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h	Tue Dec 08 17:02:15 2009 +0000
> +++ b/tools/libxl/libxl.h	Tue Dec 08 17:14:35 2009 +0000
> @@ -66,6 +66,7 @@
>      int max_vcpus;
>      int cur_vcpus;
>      uint32_t max_memkb;
> +    uint32_t target_memkb;
>      uint32_t video_memkb;
>      uint32_t shadow_memkb;
>      const char *kernel;
> @@ -305,6 +306,8 @@
>  int libxl_domain_pause(struct libxl_ctx *ctx, uint32_t domid);
>  int libxl_domain_unpause(struct libxl_ctx *ctx, uint32_t domid);
>  
> +int libxl_set_memory_target(struct libxl_ctx *ctx, uint32_t domid, uint32_t target_memkb);
> +
>  int libxl_console_attach(struct libxl_ctx *ctx, uint32_t domid, int cons_num);
>  
>  struct libxl_dominfo * libxl_domain_list(struct libxl_ctx *ctx, int *nb_domain);
> diff -r fe3c8ba27d2f tools/libxl/libxl_dom.c
> --- a/tools/libxl/libxl_dom.c	Tue Dec 08 17:02:15 2009 +0000
> +++ b/tools/libxl/libxl_dom.c	Tue Dec 08 17:14:35 2009 +0000
> @@ -52,7 +52,7 @@
>      if (info->vpt_align != -1)
>          xc_set_hvm_param(ctx->xch, domid, HVM_PARAM_VPT_ALIGN, (unsigned long) info->vpt_align);
>      xc_domain_max_vcpus(ctx->xch, domid, info->max_vcpus);
> -    xc_domain_setmaxmem(ctx->xch, domid, info->max_memkb + info->video_memkb);
> +    xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + LIBXL_MAXMEM_CONSTANT);
>      xc_domain_set_memmap_limit(ctx->xch, domid, 
>              (info->hvm) ? info->max_memkb : 
>              (info->max_memkb + info->u.pv.slack_memkb));
> @@ -81,7 +81,9 @@
>      ents[0] = "memory/static-max";
>      ents[1] = libxl_sprintf(ctx, "%d", info->max_memkb);
>      ents[2] = "memory/target";
> -    ents[3] = libxl_sprintf(ctx, "%d", info->max_memkb); /* PROBABLY WRONG */
> +    ents[3] = libxl_sprintf(ctx, "%d", info->target_memkb);
> +    ents[2] = "memory/videoram";
> +    ents[3] = libxl_sprintf(ctx, "%d", info->video_memkb);
>      ents[4] = "domid";
>      ents[5] = libxl_sprintf(ctx, "%d", domid);
>      ents[6] = "store/port";
> @@ -145,7 +147,7 @@
>  {
>      int ret;
>  
> -    ret = xc_hvm_build(ctx->xch, domid, info->max_memkb / 1024, info->kernel);
> +    ret = xc_hvm_build_target_mem(ctx->xch, domid, (info->max_memkb - info->video_memkb) / 1024, (info->target_memkb - info->video_memkb) / 1024, info->kernel);
>      if (ret) {
>          XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, ret, "hvm building failed");
>          return ERROR_FAIL;
> diff -r fe3c8ba27d2f tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h	Tue Dec 08 17:02:15 2009 +0000
> +++ b/tools/libxl/libxl_internal.h	Tue Dec 08 17:14:35 2009 +0000
> @@ -31,6 +31,7 @@
>  #define LIBXL_DEVICE_MODEL_START_TIMEOUT 10
>  #define LIBXL_XENCONSOLE_LIMIT 1048576
>  #define LIBXL_XENCONSOLE_PROTOCOL "vt100"
> +#define LIBXL_MAXMEM_CONSTANT (1 * 1024 * 1024)
>  #define QEMU_SIGNATURE "QemuDeviceModelRecord"
>  
>  #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
> diff -r fe3c8ba27d2f tools/libxl/xl.c
> --- a/tools/libxl/xl.c	Tue Dec 08 17:02:15 2009 +0000
> +++ b/tools/libxl/xl.c	Tue Dec 08 17:14:35 2009 +0000
> @@ -85,6 +85,7 @@
>      printf("vpt_align: %d\n", b_info->vpt_align);
>      printf("max_vcpus: %d\n", b_info->max_vcpus);
>      printf("max_memkb: %d\n", b_info->max_memkb);
> +    printf("target_memkb: %d\n", b_info->target_memkb);
>      printf("kernel: %s\n", b_info->kernel);
>      printf("hvm: %d\n", b_info->hvm);
>  
> @@ -283,8 +284,10 @@
>      if (config_lookup_int (&config, "vcpus", &l) == CONFIG_TRUE)
>          b_info->max_vcpus = l;
>  
> -    if (config_lookup_int (&config, "memory", &l) == CONFIG_TRUE)
> +    if (config_lookup_int (&config, "memory", &l) == CONFIG_TRUE) {
>          b_info->max_memkb = l * 1024;
> +        b_info->target_memkb = b_info->max_memkb;
> +    }
>  
>      if (config_lookup_int (&config, "shadow_memory", &l) == CONFIG_TRUE)
>          b_info->shadow_memkb = l * 1024;
> @@ -756,6 +759,7 @@
>          printf(" restore                       restore a domain from a saved state\n\n");
>          printf(" cd-insert                     insert a cdrom into a guest's cd drive\n\n");
>          printf(" cd-eject                      eject a cdrom from a guest's cd drive\n\n");
> +        printf(" mem-set                       set the current memory usage for a domain\n\n");
>      } else if(!strcmp(command, "create")) {
>          printf("Usage: xl create <ConfigFile> [options] [vars]\n\n");
>          printf("Create a domain based on <ConfigFile>.\n\n");
> @@ -804,7 +808,52 @@
>      } else if (!strcmp(command, "cd-eject")) {
>          printf("Usage: xl cd-eject <Domain> <VirtualDevice>\n\n");
>          printf("Eject a cdrom from a guest's cd drive.\n\n");
> +    } else if (!strcmp(command, "mem-set")) {
> +        printf("Usage: xl mem-set <Domain> <MemKB>\n\n");
> +        printf("Set the current memory usage for a domain.\n\n");
>      }
> +}
> +
> +void set_memory_target(char *p, char *mem)
> +{
> +    struct libxl_ctx ctx;
> +    uint32_t domid;
> +
> +    libxl_ctx_init(&ctx);
> +    libxl_ctx_set_log(&ctx, log_callback, NULL);
> +
> +    if (libxl_param_to_domid(&ctx, p, &domid) < 0) {
> +        fprintf(stderr, "%s is an invalid domain identifier\n", p);
> +        exit(2);
> +    }
> +    libxl_set_memory_target(&ctx, domid, atoi(mem));
> +}
> +
> +int main_memset(int argc, char **argv)
> +{
> +    int opt = 0;
> +    char *p = NULL, *mem;
> +
> +    while ((opt = getopt(argc, argv, "h:")) != -1) {
> +        switch (opt) {
> +        case 'h':
> +            help("mem-set");
> +            exit(0);
> +        default:
> +            fprintf(stderr, "option not supported\n");
> +            break;
> +        }
> +    }
> +    if (optind >= argc - 1) {
> +        help("mem-set");
> +        exit(2);
> +    }
> +
> +    p = argv[optind];
> +    mem = argv[optind + 1];
> +
> +    set_memory_target(p, mem);
> +    exit(0);
>  }
>  
>  void console(char *p, int cons_num)
> @@ -1436,6 +1485,8 @@
>          main_cd_insert(argc - 1, argv + 1);
>      } else if (!strcmp(argv[1], "cd-eject")) {
>          main_cd_eject(argc - 1, argv + 1);
> +    } else if (!strcmp(argv[1], "mem-set")) {
> +        main_memset(argc - 1, argv + 1);
>      } else if (!strcmp(argv[1], "help")) {
>          if (argc > 2)
>              help(argv[2]);
>
>
>
>   

Re: [PATCH] libxenlight: implement libxl_set_memory_target

[Stefano Stabellini]

On Tue, 8 Dec 2009, Andres Lagar-Cavilla wrote:
> Hi,
> couple of comments:
> - PV domains without videoram won't be able to use this

PV domains just have videoram = 0.

> - Further, doesn't the PV domain build function need to use 
> target_memkb? (That's my read of what xend does at least)

Yes, you are right, I'll fix this.

> - Finally, LIBXL_MAXMEM_CONSTANT looks like an "evil constant we should 
> avoid". Where did it come from?

I decided to introduce this constant after a discussion with developers
of the memory management functions in xapi: after thorough testing they
found that adding 1 MB to maxmem increases the robustness of the system.

BTW the current value of the constant is wrong because it should be
expressed in KB, I'll send a patch to fix this later today.

Re: [PATCH] libxenlight: implement libxl_set_memory_target

[Andres Lagar-Cavilla]

Stefano Stabellini wrote:
> On Tue, 8 Dec 2009, Andres Lagar-Cavilla wrote:
>   
>> Hi,
>> couple of comments:
>> - PV domains without videoram won't be able to use this
>>     
>
> PV domains just have videoram = 0.
>   
But you abort libxl_set_memory_target if the videoram node is not found. 
Which won't be for PVs with no videoram...
+    videoram_s = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, 
"%s/memory/videoram", dompath));
+    if (!videoram_s)
+        return -1;

Andres
>   
>> - Further, doesn't the PV domain build function need to use 
>> target_memkb? (That's my read of what xend does at least)
>>     
>
> Yes, you are right, I'll fix this.
>
>   
>> - Finally, LIBXL_MAXMEM_CONSTANT looks like an "evil constant we should 
>> avoid". Where did it come from?
>>     
>
> I decided to introduce this constant after a discussion with developers
> of the memory management functions in xapi: after thorough testing they
> found that adding 1 MB to maxmem increases the robustness of the system.
>
> BTW the current value of the constant is wrong because it should be
> expressed in KB, I'll send a patch to fix this later today.
>
>   

Re: [PATCH] libxenlight: implement libxl_set_memory_target

[Stefano Stabellini]

On Wed, 9 Dec 2009, Andres Lagar-Cavilla wrote:
> Stefano Stabellini wrote:
> > On Tue, 8 Dec 2009, Andres Lagar-Cavilla wrote:
> >   
> >> Hi,
> >> couple of comments:
> >> - PV domains without videoram won't be able to use this
> >>     
> >
> > PV domains just have videoram = 0.
> >   
> But you abort libxl_set_memory_target if the videoram node is not found. 
> Which won't be for PVs with no videoram...
> +    videoram_s = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, 
> "%s/memory/videoram", dompath));
> +    if (!videoram_s)
> +        return -1;
> 

The videoram node will be found, and the value will be 0.

Re: [PATCH] libxenlight: implement libxl_set_memory_target

[Andres Lagar-Cavilla]

Errrr, no.
http://xenbits.xen.org/xen-unstable.hg?file/8f304c003af4/tools/libxl/libxl.c 
shows that the videoram node will be set only if we are using a 
device-model. And specifically if we are in fully virtualized mode, i.e. 
dm_info->type == XENFV, i.e. *not* PV
Andres
Stefano Stabellini wrote:
> On Wed, 9 Dec 2009, Andres Lagar-Cavilla wrote:
>   
>> Stefano Stabellini wrote:
>>     
>>> On Tue, 8 Dec 2009, Andres Lagar-Cavilla wrote:
>>>   
>>>       
>>>> Hi,
>>>> couple of comments:
>>>> - PV domains without videoram won't be able to use this
>>>>     
>>>>         
>>> PV domains just have videoram = 0.
>>>   
>>>       
>> But you abort libxl_set_memory_target if the videoram node is not found. 
>> Which won't be for PVs with no videoram...
>> +    videoram_s = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, 
>> "%s/memory/videoram", dompath));
>> +    if (!videoram_s)
>> +        return -1;
>>
>>     
>
> The videoram node will be found, and the value will be 0.
>
>   

Re: [PATCH] libxenlight: implement libxl_set_memory_target

[Stefano Stabellini]

On Thu, 10 Dec 2009, Andres Lagar-Cavilla wrote:
> Errrr, no.
> http://xenbits.xen.org/xen-unstable.hg?file/8f304c003af4/tools/libxl/libxl.c 
> shows that the videoram node will be set only if we are using a 
> device-model. And specifically if we are in fully virtualized mode, i.e. 
> dm_info->type == XENFV, i.e. *not* PV

I understand that this can be confusing, but at the moment we have two
videoram parameters: dm_info->videoram is for the device model while
b_info->video_memkb is for domain building.
If you look at init_dm_info, you'll see that dm_info->videoram is set
from b_info->video_memkb.
Also if you read libxl_dom.c:build_post you'll see that no matter if the
guest is a pv or an hvm domain, memory/videoram will be written to
xenstore anyway.

Re: [PATCH] libxenlight: implement libxl_set_memory_target

[Andres Lagar-Cavilla]

Oh, I'm working with an old changeset. Yikes. Thanks,
Andres
Stefano Stabellini wrote:
> On Thu, 10 Dec 2009, Andres Lagar-Cavilla wrote:
>   
>> Errrr, no.
>> http://xenbits.xen.org/xen-unstable.hg?file/8f304c003af4/tools/libxl/libxl.c 
>> shows that the videoram node will be set only if we are using a 
>> device-model. And specifically if we are in fully virtualized mode, i.e. 
>> dm_info->type == XENFV, i.e. *not* PV
>>     
>
> I understand that this can be confusing, but at the moment we have two
> videoram parameters: dm_info->videoram is for the device model while
> b_info->video_memkb is for domain building.
> If you look at init_dm_info, you'll see that dm_info->videoram is set
> from b_info->video_memkb.
> Also if you read libxl_dom.c:build_post you'll see that no matter if the
> guest is a pv or an hvm domain, memory/videoram will be written to
> xenstore anyway.
>
>