From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:59108)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <sw@weilnetz.de>) id 1XK1e4-0000Qn-LK
	for qemu-devel@nongnu.org; Wed, 20 Aug 2014 04:53:34 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <sw@weilnetz.de>) id 1XK1dz-0005dV-2l
	for qemu-devel@nongnu.org; Wed, 20 Aug 2014 04:53:28 -0400
Received: from v220110690675601.yourvserver.net ([37.221.199.173]:44545)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <sw@weilnetz.de>) id 1XK1dy-0005dP-Od
	for qemu-devel@nongnu.org; Wed, 20 Aug 2014 04:53:23 -0400
Message-ID: <53F461FD.70900@weilnetz.de>
Date: Wed, 20 Aug 2014 10:53:17 +0200
From: Stefan Weil <sw@weilnetz.de>
MIME-Version: 1.0
References: <cover.1408508297.git.peter.crosthwaite@xilinx.com>
	<2159303c366ca757eacbcc1201e946057df4ba9e.1408508297.git.peter.crosthwaite@xilinx.com>
In-Reply-To: <2159303c366ca757eacbcc1201e946057df4ba9e.1408508297.git.peter.crosthwaite@xilinx.com>
Content-Type: text/plain; charset=iso-8859-15
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH bugfix v1 1/3] xen: hvm: Abstract away
 memory region name ref (fix broken build)
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Peter Crosthwaite <peter.crosthwaite@xilinx.com>, qemu-devel@nongnu.org, peter.maydell@linaro.org
Cc: pbonzini@redhat.com

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;