[PATCH] - return error when appropriate from increase_memory_reservation

[PATCH] - return error when appropriate from increase_memory_reservation

[Ben Thomas]

[-- Attachment #1: Type: text/plain, Size: 1004 bytes --]

This patch fixes a case in increasing a memory reservation where you
do not get the pages nor do you get an error.  While an argument might
be made that checking the page count independently is workable, it
does seem reasonable to have the operation return a failure in the
cases where it doesn't do what was asked. Right now, it mostly returns
the correct status.  This patch just adds another instance of this.

And, while-I-was-there, I've been generating linker maps for qemu in
my own builds. It's harmless, until you need it and then it's valuable.
Tweak the Makefile to create the map.

Signed-off-by:  Ben Thomas (ben@virtualiron.com)

-- 
------------------------------------------------------------------------
Ben Thomas                                         Virtual Iron Software
bthomas@virtualiron.com                            Tower 1, Floor 2
978-849-1214                                       900 Chelmsford Street
                                                    Lowell, MA 01851

[-- Attachment #2: res.txt --]
[-- Type: text/plain, Size: 955 bytes --]

--- a/tools/ioemu/target-i386-dm/Makefile	Fri Apr 14 14:31:13 2006 -0400
+++ b/tools/ioemu/target-i386-dm/Makefile	Thu Apr 20 13:56:56 2006 -0400
@@ -323,7 +323,7 @@ endif
 endif
 
 $(QEMU_SYSTEM): $(VL_OBJS) libqemu.a
-	$(CC) $(CFLAGS) $(VL_LDFLAGS) -o $@ $^ $(LIBS) $(SDL_LIBS) $(VNC_LIBS) $(VL_LIBS) -lpthread
+	$(CC) $(CFLAGS) $(VL_LDFLAGS) -o $@ $^ $(LIBS) $(SDL_LIBS) $(VNC_LIBS) $(VL_LIBS) -lpthread -Wl,-Map,qemu-dm.map -Wl,--cref
 
 vnc.o: vnc.c keyboard_rdesktop.c
 	$(CC) $(CFLAGS) $(DEFINES) $(VNC_CFLAGS) -c -o $@ $<
diff -r 487f5e5c0fbd xen/common/memory.c
--- a/xen/common/memory.c	Fri Apr 14 14:31:13 2006 -0400
+++ b/xen/common/memory.c	Thu Apr 20 13:56:56 2006 -0400
@@ -348,6 +348,9 @@ long do_memory_op(unsigned long cmd, GUE
             break;
         }
 
+        if (rc < reservation.nr_extents && !preempted)
+            return -ENOMEM;
+ 
         if ( unlikely(reservation.domid != DOMID_SELF) )
             put_domain(d);
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] - return error when appropriate from increase_memory_reservation

[Keir Fraser]

On 20 Apr 2006, at 19:04, Ben Thomas wrote:

> This patch fixes a case in increasing a memory reservation where you
> do not get the pages nor do you get an error.  While an argument might
> be made that checking the page count independently is workable, it
> does seem reasonable to have the operation return a failure in the
> cases where it doesn't do what was asked. Right now, it mostly returns
> the correct status.  This patch just adds another instance of this.

No. In cases where it fails it does not undo its partial work. The 
current return lets the caller know how much work was done so that 
appropriate action can be taken. The callers need fixing -- there 
aren't that many. Only increase_reservation or populate_physmap can 
fail, unless the caller is buggy, so they are the only ones that really 
need fixing. The others should probably BUG_ON() or assert or print an 
error in the caller though.

> And, while-I-was-there, I've been generating linker maps for qemu in
> my own builds. It's harmless, until you need it and then it's valuable.
> Tweak the Makefile to create the map.

I'll take that as a separate patch.

  Thanks,
  Keir

Re: [PATCH] - return error when appropriate from increase_memory_reservation

[Ben Thomas]

You're right - I was fixing this too quickly. The issue I was
encountering is that xc_domain_memory_increase_reservation
may return 0(success) when no pages have been reserved.  Put
this in a loop and you get, well, interesting results.

There are several possible solutions for this, all done at
that level.  However, it isn't clear to me at this moment
what the best and most correct solution would be. So, I'm
going to put this off for now and continue to use a different
approach.

Thanks,
-b

Keir Fraser wrote:
> 
> On 20 Apr 2006, at 19:04, Ben Thomas wrote:
> 
>> This patch fixes a case in increasing a memory reservation where you
>> do not get the pages nor do you get an error.  While an argument might
>> be made that checking the page count independently is workable, it
>> does seem reasonable to have the operation return a failure in the
>> cases where it doesn't do what was asked. Right now, it mostly returns
>> the correct status.  This patch just adds another instance of this.
> 
> 
> No. In cases where it fails it does not undo its partial work. The 
> current return lets the caller know how much work was done so that 
> appropriate action can be taken. The callers need fixing -- there aren't 
> that many. Only increase_reservation or populate_physmap can fail, 
> unless the caller is buggy, so they are the only ones that really need 
> fixing. The others should probably BUG_ON() or assert or print an error 
> in the caller though.
> 
>> And, while-I-was-there, I've been generating linker maps for qemu in
>> my own builds. It's harmless, until you need it and then it's valuable.
>> Tweak the Makefile to create the map.
> 
> 
> I'll take that as a separate patch.
> 
>  Thanks,
>  Keir
> 


-- 
------------------------------------------------------------------------
Ben Thomas                                         Virtual Iron Software
bthomas@virtualiron.com                            Tower 1, Floor 2
978-849-1214                                       900 Chelmsford Street
                                                    Lowell, MA 01851