Re: [Qemu-devel] [PATCH bugfix v1 1/3] xen: hvm: Abstract away memory region name ref (fix broken build)

[Stefan Weil <sw@weilnetz.de>]

Am 20.08.2014 um 07:07 schrieb Peter Crosthwaite:
> The mr->name field is removed. This slipped through compile testing.
> Fix.
> 
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
> 
>  xen-hvm.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/xen-hvm.c b/xen-hvm.c
> index 91de2e2..9fd9d6e 100644
> --- a/xen-hvm.c
> +++ b/xen-hvm.c
> @@ -291,6 +291,7 @@ static int xen_add_to_physmap(XenIOState *state,
>      hwaddr pfn, start_gpfn;
>      hwaddr phys_offset = memory_region_get_ram_addr(mr);
>      char path[80], value[17];
> +    const char *mr_name;
>  
>      if (get_physmapping(state, start_addr, size)) {
>          return 0;
> @@ -326,11 +327,13 @@ go_physmap:
>          }
>      }
>  
> +    mr_name = memory_region_name(mr);
> +
>      physmap = g_malloc(sizeof (XenPhysmap));
>  
>      physmap->start_addr = start_addr;
>      physmap->size = size;
> -    physmap->name = (char *)mr->name;
> +    physmap->name = (char *)mr_name;

This type cast can be removed. Just add the const in XenPhysmap's name
declaration.

>      physmap->phys_offset = phys_offset;
>  
>      QLIST_INSERT_HEAD(&state->physmap, physmap, list);
> @@ -354,11 +357,11 @@ go_physmap:
>      if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
>          return -1;
>      }
> -    if (mr->name) {
> +    if (mr_name) {
>          snprintf(path, sizeof(path),
>                  "/local/domain/0/device-model/%d/physmap/%"PRIx64"/name",
>                  xen_domid, (uint64_t)phys_offset);
> -        if (!xs_write(state->xenstore, 0, path, mr->name, strlen(mr->name))) {
> +        if (!xs_write(state->xenstore, 0, path, mr_name, strlen(mr_name))) {
>              return -1;
>          }
>      }
> 


Otherwise ok. Below is my own variant which I tested before I noticed
your patch.

Reviewed-by: Stefan Weil <sw@weilnetz.de>


@@ -69,11 +69,11 @@ static inline ioreq_t
*xen_vcpu_ioreq(shared_iopage_t *shared_page, int vcpu)
 #define BUFFER_IO_MAX_DELAY  100

 typedef struct XenPhysmap {
     hwaddr start_addr;
     ram_addr_t size;
-    char *name;
+    const char *name;
     hwaddr phys_offset;

     QLIST_ENTRY(XenPhysmap) list;
 } XenPhysmap;

@@ -328,11 +328,11 @@ go_physmap:

     physmap = g_malloc(sizeof (XenPhysmap));

     physmap->start_addr = start_addr;
     physmap->size = size;
-    physmap->name = (char *)mr->name;
+    physmap->name = memory_region_name(mr);
     physmap->phys_offset = phys_offset;

     QLIST_INSERT_HEAD(&state->physmap, physmap, list);

     xc_domain_pin_memory_cacheattr(xen_xc, xen_domid,
@@ -352,15 +352,16 @@ go_physmap:
             xen_domid, (uint64_t)phys_offset);
     snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)size);
     if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
         return -1;
     }
-    if (mr->name) {
+    if (physmap->name) {
         snprintf(path, sizeof(path),
                 "/local/domain/0/device-model/%d/physmap/%"PRIx64"/name",
                 xen_domid, (uint64_t)phys_offset);
-        if (!xs_write(state->xenstore, 0, path, mr->name,
strlen(mr->name))) {
+        if (!xs_write(state->xenstore, 0, path, physmap->name,
+                      strlen(physmap->name))) {
             return -1;
         }
     }

     return 0;