public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] qemu: use statically allocate 512 byte buffer in the stack for sector in bdrv_commit
@ 2008-01-04  7:11 Carlo Marcelo Arenas Belon
  2008-01-07  9:27 ` Avi Kivity
  0 siblings, 1 reply; 15+ messages in thread
From: Carlo Marcelo Arenas Belon @ 2008-01-04  7:11 UTC (permalink / raw)
  To: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

revert a merge conflict from 075da586c92f09bd9a7401f1e80d72fde27c173 that
redefined sector as an array of pointers to char, instead of a statically
allocated buffer of chars, that was triggering the following warnings :

block.c: In function `bdrv_commit':
block.c:480: warning: passing arg 3 of `bdrv_read' from incompatible pointer type
block.c:484: warning: passing arg 3 of `bdrv_write' from incompatible pointer type

Signed-off-by: Carlo Marcelo Arenas Belon <carenas-kLeDWSohozoJb6fo7hG9ng@public.gmane.org>
---
 qemu/block.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/qemu/block.c b/qemu/block.c
index 519be24..492a4d3 100644
--- a/qemu/block.c
+++ b/qemu/block.c
@@ -460,7 +460,7 @@ int bdrv_commit(BlockDriverState *bs)
     BlockDriver *drv = bs->drv;
     int64_t i, total_sectors;
     int n, j;
-    unsigned char *sector[512];
+    unsigned char sector[512];
 
     if (!drv)
         return -ENOMEDIUM;
-- 
1.5.3.7


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] qemu: use statically allocate 512 byte buffer in the stack for sector in bdrv_commit
  2008-01-04  7:11 [PATCH] qemu: use statically allocate 512 byte buffer in the stack for sector in bdrv_commit Carlo Marcelo Arenas Belon
@ 2008-01-07  9:27 ` Avi Kivity
       [not found]   ` <4781F08E.8060407-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Avi Kivity @ 2008-01-07  9:27 UTC (permalink / raw)
  To: Carlo Marcelo Arenas Belon
  Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Laurent Vivier

Carlo Marcelo Arenas Belon wrote:
> revert a merge conflict from 075da586c92f09bd9a7401f1e80d72fde27c173 that
> redefined sector as an array of pointers to char, instead of a statically
> allocated buffer of chars, that was triggering the following warnings :
>
> block.c: In function `bdrv_commit':
> block.c:480: warning: passing arg 3 of `bdrv_read' from incompatible pointer type
> block.c:484: warning: passing arg 3 of `bdrv_write' from incompatible pointer type
>
> Signed-off-by: Carlo Marcelo Arenas Belon <carenas-kLeDWSohozoJb6fo7hG9ng@public.gmane.org>
>   

Doesn't the cache=off option warrant an allocation here to ensure 
alignment (or perhaps a 1K stack buffer with runtime adjustment)?

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] qemu: use statically allocate 512 byte buffer in the stack for sector in bdrv_commit
       [not found]   ` <4781F08E.8060407-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
@ 2008-01-07 10:22     ` Laurent Vivier
  2008-01-07 10:47       ` Avi Kivity
  0 siblings, 1 reply; 15+ messages in thread
From: Laurent Vivier @ 2008-01-07 10:22 UTC (permalink / raw)
  To: Avi Kivity
  Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Carlo Marcelo Arenas Belon


[-- Attachment #1.1: Type: text/plain, Size: 2483 bytes --]

Le lundi 07 janvier 2008 à 11:27 +0200, Avi Kivity a écrit :
> Carlo Marcelo Arenas Belon wrote:
> > revert a merge conflict from 075da586c92f09bd9a7401f1e80d72fde27c173 that
> > redefined sector as an array of pointers to char, instead of a statically
> > allocated buffer of chars, that was triggering the following warnings :
> >
> > block.c: In function `bdrv_commit':
> > block.c:480: warning: passing arg 3 of `bdrv_read' from incompatible pointer type
> > block.c:484: warning: passing arg 3 of `bdrv_write' from incompatible pointer type
> >
> > Signed-off-by: Carlo Marcelo Arenas Belon <carenas-kLeDWSohozoJb6fo7hG9ng@public.gmane.org>
> >   
> 
> Doesn't the cache=off option warrant an allocation here to ensure 
> alignment (or perhaps a 1K stack buffer with runtime adjustment)?

You're right, a good patch should be something like this (it is not
tested or even compiled) :

Signed-off-by: Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org>
diff --git a/qemu/block.c b/qemu/block.c
index 519be24..c198659 100644
--- a/qemu/block.c
+++ b/qemu/block.c
@@ -460,7 +460,7 @@ int bdrv_commit(BlockDriverState *bs)
     BlockDriver *drv = bs->drv;
     int64_t i, total_sectors;
     int n, j;
-    unsigned char *sector[512];
+    unsigned char *sector;
 
     if (!drv)
         return -ENOMEDIUM;
@@ -473,15 +473,21 @@ int bdrv_commit(BlockDriverState *bs)
        return -ENOTSUP;
     }
 
+    sector = qemu_memalign(512,512);
+    if (sector == NULL)
+        return -ENOMEM;
+
     total_sectors = bdrv_getlength(bs) >> SECTOR_BITS;
     for (i = 0; i < total_sectors;) {
         if (drv->bdrv_is_allocated(bs, i, 65536, &n)) {
             for(j = 0; j < n; j++) {
                 if (bdrv_read(bs, i, sector, 1) != 0) {
+                    qemu_free(sector);
                     return -EIO;
                 }
 
                 if (bdrv_write(bs->backing_hd, i, sector, 1) != 0) {
+                    qemu_free(sector);
                     return -EIO;
                 }
                 i++;
@@ -491,6 +497,7 @@ int bdrv_commit(BlockDriverState *bs)
         }
     }
 
+    qemu_free(sector);
     if (drv->bdrv_make_empty)
        return drv->bdrv_make_empty(bs);

-- 
----------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #1.2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 228 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

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

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] qemu: use statically allocate 512 byte buffer in the stack for sector in bdrv_commit
  2008-01-07 10:22     ` Laurent Vivier
@ 2008-01-07 10:47       ` Avi Kivity
       [not found]         ` <4782034C.4000805-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Avi Kivity @ 2008-01-07 10:47 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Carlo Marcelo Arenas Belon

Laurent Vivier wrote:
> Le lundi 07 janvier 2008 à 11:27 +0200, Avi Kivity a écrit :
>   
>> Carlo Marcelo Arenas Belon wrote:
>>     
>>> revert a merge conflict from 075da586c92f09bd9a7401f1e80d72fde27c173 that
>>> redefined sector as an array of pointers to char, instead of a statically
>>> allocated buffer of chars, that was triggering the following warnings :
>>>
>>> block.c: In function `bdrv_commit':
>>> block.c:480: warning: passing arg 3 of `bdrv_read' from incompatible pointer type
>>> block.c:484: warning: passing arg 3 of `bdrv_write' from incompatible pointer type
>>>
>>> Signed-off-by: Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe>
>>>   
>>>       
>> Doesn't the cache=off option warrant an allocation here to ensure 
>> alignment (or perhaps a 1K stack buffer with runtime adjustment)?
>>     
>
> You're right, a good patch should be something like this (it is not
> tested or even compiled) :
>   

Looks good, but patch is corrupted by mail client.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] qemu: use statically allocate 512 byte buffer in the stack for sector in bdrv_commit
       [not found]         ` <4782034C.4000805-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
@ 2008-01-07 12:08           ` Laurent Vivier
  2008-01-07 15:16           ` Laurent Vivier
  1 sibling, 0 replies; 15+ messages in thread
From: Laurent Vivier @ 2008-01-07 12:08 UTC (permalink / raw)
  To: Avi Kivity
  Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Carlo Marcelo Arenas Belon


[-- Attachment #1.1: Type: text/plain, Size: 1436 bytes --]

Le lundi 07 janvier 2008 à 12:47 +0200, Avi Kivity a écrit :
> Laurent Vivier wrote:
> > Le lundi 07 janvier 2008 à 11:27 +0200, Avi Kivity a écrit :
> >   
> >> Carlo Marcelo Arenas Belon wrote:
> >>     
> >>> revert a merge conflict from 075da586c92f09bd9a7401f1e80d72fde27c173 that
> >>> redefined sector as an array of pointers to char, instead of a statically
> >>> allocated buffer of chars, that was triggering the following warnings :
> >>>
> >>> block.c: In function `bdrv_commit':
> >>> block.c:480: warning: passing arg 3 of `bdrv_read' from incompatible pointer type
> >>> block.c:484: warning: passing arg 3 of `bdrv_write' from incompatible pointer type
> >>>
> >>> Signed-off-by: Carlo Marcelo Arenas Belon <carenas-kLeDWSohozoJb6fo7hG9ng@public.gmane.org>
> >>>   
> >>>       
> >> Doesn't the cache=off option warrant an allocation here to ensure 
> >> alignment (or perhaps a 1K stack buffer with runtime adjustment)?
> >>     
> >
> > You're right, a good patch should be something like this (it is not
> > tested or even compiled) :
> >   
> 
> Looks good, but patch is corrupted by mail client.

OK, I'll resend it later (I'd like to compile it at least).

Laurent
-- 
----------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #1.2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 228 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

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

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] qemu: use statically allocate 512 byte buffer in the stack for sector in bdrv_commit
       [not found]         ` <4782034C.4000805-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
  2008-01-07 12:08           ` Laurent Vivier
@ 2008-01-07 15:16           ` Laurent Vivier
  2008-01-07 15:34             ` Javier Guerra
  2008-01-07 18:27             ` Avi Kivity
  1 sibling, 2 replies; 15+ messages in thread
From: Laurent Vivier @ 2008-01-07 15:16 UTC (permalink / raw)
  To: Avi Kivity
  Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Carlo Marcelo Arenas Belon


[-- Attachment #1.1: Type: text/plain, Size: 1599 bytes --]

Le lundi 07 janvier 2008 à 12:47 +0200, Avi Kivity a écrit :
> Laurent Vivier wrote:
> > Le lundi 07 janvier 2008 à 11:27 +0200, Avi Kivity a écrit :
> >   
> >> Carlo Marcelo Arenas Belon wrote:
> >>     
> >>> revert a merge conflict from 075da586c92f09bd9a7401f1e80d72fde27c173 that
> >>> redefined sector as an array of pointers to char, instead of a statically
> >>> allocated buffer of chars, that was triggering the following warnings :
> >>>
> >>> block.c: In function `bdrv_commit':
> >>> block.c:480: warning: passing arg 3 of `bdrv_read' from incompatible pointer type
> >>> block.c:484: warning: passing arg 3 of `bdrv_write' from incompatible pointer type
> >>>
> >>> Signed-off-by: Carlo Marcelo Arenas Belon <carenas-kLeDWSohozoJb6fo7hG9ng@public.gmane.org>
> >>>   
> >>>       
> >> Doesn't the cache=off option warrant an allocation here to ensure 
> >> alignment (or perhaps a 1K stack buffer with runtime adjustment)?
> >>     
> >
> > You're right, a good patch should be something like this (it is not
> > tested or even compiled) :
> >   
> 
> Looks good, but patch is corrupted by mail client.

What I'm wondering now is: is it really useful to have "cache=off" and
"snapshot=on" at the same time ?
If not, the patch of Carlo is good, otherwise there is more
modifications to do (in other parts of qemu).

Regards,
Laurent
-- 
----------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #1.2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 228 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

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

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] qemu: use statically allocate 512 byte buffer in the stack for sector in bdrv_commit
  2008-01-07 15:16           ` Laurent Vivier
@ 2008-01-07 15:34             ` Javier Guerra
       [not found]               ` <90eb1dc70801070734l2062cac6r7a7bed1d6d3d2c0c-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2008-01-07 18:27             ` Avi Kivity
  1 sibling, 1 reply; 15+ messages in thread
From: Javier Guerra @ 2008-01-07 15:34 UTC (permalink / raw)
  To: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On 1/7/08, Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org> wrote:
> What I'm wondering now is: is it really useful to have "cache=off" and
> "snapshot=on" at the same time ?

does "cache=off" means disk cache? if so, it might be useful to test
clustering filesystems.

so far, the only way is to setup a network block device (iSCSI, AoE,
nbd). i'd like to simply specify the same backing file for two
instances' hdb parameter.

and snapshots help a lot to go back after blowing up the on-disk structures


-- 
Javier

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] qemu: use statically allocate 512 byte buffer in the stack for sector in bdrv_commit
       [not found]               ` <90eb1dc70801070734l2062cac6r7a7bed1d6d3d2c0c-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2008-01-07 15:45                 ` Laurent Vivier
  2008-01-07 16:03                   ` Javier Guerra
  2008-01-07 18:29                 ` Avi Kivity
  1 sibling, 1 reply; 15+ messages in thread
From: Laurent Vivier @ 2008-01-07 15:45 UTC (permalink / raw)
  To: Javier Guerra; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f


[-- Attachment #1.1: Type: text/plain, Size: 1182 bytes --]

Le lundi 07 janvier 2008 à 10:34 -0500, Javier Guerra a écrit :
> On 1/7/08, Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org> wrote:
> > What I'm wondering now is: is it really useful to have "cache=off" and
> > "snapshot=on" at the same time ?
> 
> does "cache=off" means disk cache? if so, it might be useful to test
> clustering filesystems.

"cache=off" means files is opened with "O_DIRECT" and thus there is no
cache in the kernel memory on the host side.
IMO, "cache=off" and "snapshot=on" are incompatible because a snapshot
can be seen like a cache.

> so far, the only way is to setup a network block device (iSCSI, AoE,
> nbd). i'd like to simply specify the same backing file for two
> instances' hdb parameter.

I'm sorry but I don't understand this part.

> and snapshots help a lot to go back after blowing up the on-disk structures

But I think if you use a snapshot there is no reason to use "cache=off"

Laurent
-- 
----------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #1.2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 228 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

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

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] qemu: use statically allocate 512 byte buffer in the stack for sector in bdrv_commit
  2008-01-07 15:45                 ` Laurent Vivier
@ 2008-01-07 16:03                   ` Javier Guerra
       [not found]                     ` <90eb1dc70801070803w1a863acs75677e707446f79a-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Javier Guerra @ 2008-01-07 16:03 UTC (permalink / raw)
  To: Laurent Vivier, kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On 1/7/08, Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org> wrote:
> "cache=off" means files is opened with "O_DIRECT" and thus there is no
> cache in the kernel memory on the host side.
> IMO, "cache=off" and "snapshot=on" are incompatible because a snapshot
> can be seen like a cache.
>
> > so far, the only way is to setup a network block device (iSCSI, AoE,
> > nbd). i'd like to simply specify the same backing file for two
> > instances' hdb parameter.
>
> I'm sorry but I don't understand this part.

to test a cluster filesystem, i need two (virtual) machines with some
shared storage.  i tried long ago something like this (with (k)qemu):

- create a disk image, call it hda-1.img
- boot and install linux on it, shutdown
- copy to hda-2.img
- boot it (with new MAC) and change IP, hostname, little things, shutdown
- boot both with the same bridge, check that network works between them
- create a new disk image, call id hdb-shr.img
- boot both VMs, sharing hdb-shr.img:

qemu -hda=hda-1.img -hdb-shr.img
qemu -hda=hda-2.img -hdb-shr.img

- try to setup a cluster filesystem on hdb

it almost worked... but some writes didn't propagate to the other
until some extra writes to hdb; so i guessed that each qemu instance
had some caching on file I/O

hopefully, it would now work with "-cache=off", don't you think?

> > and snapshots help a lot to go back after blowing up the on-disk structures
>
> But I think if you use a snapshot there is no reason to use "cache=off"

in the above case, if both KVM instances share the snapshot without
cacheing it, the cluster should still work, and have some rollback
capability at the same time

or is it too much wishful thinking?

-- 
Javier

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] qemu: use statically allocate 512 byte buffer in the stack for sector in bdrv_commit
       [not found]                     ` <90eb1dc70801070803w1a863acs75677e707446f79a-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2008-01-07 16:30                       ` Laurent Vivier
  2008-01-07 16:42                         ` Javier Guerra
  2008-01-07 18:32                       ` Avi Kivity
  1 sibling, 1 reply; 15+ messages in thread
From: Laurent Vivier @ 2008-01-07 16:30 UTC (permalink / raw)
  To: Javier Guerra; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f


[-- Attachment #1.1: Type: text/plain, Size: 2660 bytes --]

Le lundi 07 janvier 2008 à 11:03 -0500, Javier Guerra a écrit :
> On 1/7/08, Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org> wrote:
> > "cache=off" means files is opened with "O_DIRECT" and thus there is no
> > cache in the kernel memory on the host side.
> > IMO, "cache=off" and "snapshot=on" are incompatible because a snapshot
> > can be seen like a cache.
> >
> > > so far, the only way is to setup a network block device (iSCSI, AoE,
> > > nbd). i'd like to simply specify the same backing file for two
> > > instances' hdb parameter.
> >
> > I'm sorry but I don't understand this part.
> 
> to test a cluster filesystem, i need two (virtual) machines with some
> shared storage.  i tried long ago something like this (with (k)qemu):
> 
> - create a disk image, call it hda-1.img
> - boot and install linux on it, shutdown
> - copy to hda-2.img
> - boot it (with new MAC) and change IP, hostname, little things, shutdown
> - boot both with the same bridge, check that network works between them
> - create a new disk image, call id hdb-shr.img
> - boot both VMs, sharing hdb-shr.img:
> 
> qemu -hda=hda-1.img -hdb-shr.img
> qemu -hda=hda-2.img -hdb-shr.img
> 
> - try to setup a cluster filesystem on hdb
> 
> it almost worked... but some writes didn't propagate to the other
> until some extra writes to hdb; so i guessed that each qemu instance
> had some caching on file I/O
> 
> hopefully, it would now work with "-cache=off", don't you think?

Well, I don't think the problem is at the host level but at the guest
level, because both instances of qemu share the host cache and thus
first instance should see changes made by the second instance (and
vice-versa).
There are also some caches at qemu level to emulate DMA, for instance in
hw/ide.c it is MAX_MULT_SECTORS (16) which is 8 kB buffer, perhaps your
problem is here but "cache=off" doesn't remove this.
Did you try to change MAX_MULT_SECTORS to 1 ?

What do you call a "cluster filesystem" ?
 
> > > and snapshots help a lot to go back after blowing up the on-disk structures
> >
> > But I think if you use a snapshot there is no reason to use "cache=off"
> 
> in the above case, if both KVM instances share the snapshot without
> cacheing it, the cluster should still work, and have some rollback
> capability at the same time
> 
> or is it too much wishful thinking?

Unfortunately I guess...

Laurent
-- 
----------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #1.2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 228 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

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

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] qemu: use statically allocate 512 byte buffer in the stack for sector in bdrv_commit
  2008-01-07 16:30                       ` Laurent Vivier
@ 2008-01-07 16:42                         ` Javier Guerra
       [not found]                           ` <90eb1dc70801070842g585cd92dr8a8a383e2f3274df-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Javier Guerra @ 2008-01-07 16:42 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On 1/7/08, Laurent Vivier <Laurent.Vivier@bull.net> wrote:
> Le lundi 07 janvier 2008 à 11:03 -0500, Javier Guerra a écrit :
> > hopefully, it would now work with "-cache=off", don't you think?
>
> Well, I don't think the problem is at the host level but at the guest
> level, because both instances of qemu share the host cache and thus
> first instance should see changes made by the second instance (and
> vice-versa).

that's what a cluster filesystem is designed to cope with (and in fact expects)

> There are also some caches at qemu level to emulate DMA, for instance in
> hw/ide.c it is MAX_MULT_SECTORS (16) which is 8 kB buffer, perhaps your
> problem is here but "cache=off" doesn't remove this.
> Did you try to change MAX_MULT_SECTORS to 1 ?

nope, don't know enough of qemu internals...

but if those caches can be flushed from the guest, the filesystem
should do that when writing its metadata

> What do you call a "cluster filesystem" ?

GFS, OCFS2, etc.   that is, filesystems that are designed to run on
more than one host with shared block storage.  usually that means
FibreChannel or iSCSI, but on VMs, a common backing could work too (at
least in theory).

Xen manages it, at least on paravirtualized guests; hadn't tried on
HVM guests. maybe the IDE-like emulation is too poor an interface to
handle it.

-- 
Javier
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] qemu: use statically allocate 512 byte buffer in the stack for sector in bdrv_commit
       [not found]                           ` <90eb1dc70801070842g585cd92dr8a8a383e2f3274df-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2008-01-07 17:05                             ` Laurent Vivier
  0 siblings, 0 replies; 15+ messages in thread
From: Laurent Vivier @ 2008-01-07 17:05 UTC (permalink / raw)
  To: Javier Guerra; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f


[-- Attachment #1.1: Type: text/plain, Size: 2000 bytes --]

Well, in fact, I think you can't use snapshot with cluster filesystem:
as each qemu instance will write in its own snapshot and will not see
modifications made by other, and I don't think there is currently a way
to share snapshot between qemu instances.

Laurent

Le lundi 07 janvier 2008 à 11:42 -0500, Javier Guerra a écrit :
> On 1/7/08, Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org> wrote:
> > Le lundi 07 janvier 2008 à 11:03 -0500, Javier Guerra a écrit :
> > > hopefully, it would now work with "-cache=off", don't you think?
> >
> > Well, I don't think the problem is at the host level but at the guest
> > level, because both instances of qemu share the host cache and thus
> > first instance should see changes made by the second instance (and
> > vice-versa).
> 
> that's what a cluster filesystem is designed to cope with (and in fact expects)
> 
> > There are also some caches at qemu level to emulate DMA, for instance in
> > hw/ide.c it is MAX_MULT_SECTORS (16) which is 8 kB buffer, perhaps your
> > problem is here but "cache=off" doesn't remove this.
> > Did you try to change MAX_MULT_SECTORS to 1 ?
> 
> nope, don't know enough of qemu internals...
> 
> but if those caches can be flushed from the guest, the filesystem
> should do that when writing its metadata
> 
> > What do you call a "cluster filesystem" ?
> 
> GFS, OCFS2, etc.   that is, filesystems that are designed to run on
> more than one host with shared block storage.  usually that means
> FibreChannel or iSCSI, but on VMs, a common backing could work too (at
> least in theory).
> 
> Xen manages it, at least on paravirtualized guests; hadn't tried on
> HVM guests. maybe the IDE-like emulation is too poor an interface to
> handle it.
> 
-- 
----------------- Laurent.Vivier-6ktuUTfB/bM@public.gmane.org  ------------------
  "La perfection est atteinte non quand il ne reste rien à
ajouter mais quand il ne reste rien à enlever." Saint Exupéry

[-- Attachment #1.2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 278 bytes --]

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

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

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] qemu: use statically allocate 512 byte buffer in the stack for sector in bdrv_commit
  2008-01-07 15:16           ` Laurent Vivier
  2008-01-07 15:34             ` Javier Guerra
@ 2008-01-07 18:27             ` Avi Kivity
  1 sibling, 0 replies; 15+ messages in thread
From: Avi Kivity @ 2008-01-07 18:27 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: kvm-devel, Carlo Marcelo Arenas Belon

Laurent Vivier wrote:
> What I'm wondering now is: is it really useful to have "cache=off" and
> "snapshot=on" at the same time ?
>   

No idea, but wouldn't want to rule it out.

> If not, the patch of Carlo is good, otherwise there is more
> modifications to do (in other parts of qemu).
>   

If other modifications are needed, perhaps do them in the I/O submission 
path (if unaligned and cache=off, force alignment).  Since most of the 
data will be aligned by the guest, the overhead will only be incurred by 
the rare metadata accesses.


-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] qemu: use statically allocate 512 byte buffer in the stack for sector in bdrv_commit
       [not found]               ` <90eb1dc70801070734l2062cac6r7a7bed1d6d3d2c0c-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2008-01-07 15:45                 ` Laurent Vivier
@ 2008-01-07 18:29                 ` Avi Kivity
  1 sibling, 0 replies; 15+ messages in thread
From: Avi Kivity @ 2008-01-07 18:29 UTC (permalink / raw)
  To: Javier Guerra; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

Javier Guerra wrote:
> On 1/7/08, Laurent Vivier <Laurent.Vivier-6ktuUTfB/bM@public.gmane.org> wrote:
>   
>> What I'm wondering now is: is it really useful to have "cache=off" and
>> "snapshot=on" at the same time ?
>>     
>
> does "cache=off" means disk cache? if so, it might be useful to test
> clustering filesystems.
>
> so far, the only way is to setup a network block device (iSCSI, AoE,
> nbd). i'd like to simply specify the same backing file for two
> instances' hdb parameter.
>   

For that to work you need to use the raw format (and then, cache=off 
doesn't matter, as a clustering filesystem ought to have a coherent cache).


-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] qemu: use statically allocate 512 byte buffer in the stack for sector in bdrv_commit
       [not found]                     ` <90eb1dc70801070803w1a863acs75677e707446f79a-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  2008-01-07 16:30                       ` Laurent Vivier
@ 2008-01-07 18:32                       ` Avi Kivity
  1 sibling, 0 replies; 15+ messages in thread
From: Avi Kivity @ 2008-01-07 18:32 UTC (permalink / raw)
  To: Javier Guerra; +Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Laurent Vivier

Javier Guerra wrote:
> to test a cluster filesystem, i need two (virtual) machines with some
> shared storage.  i tried long ago something like this (with (k)qemu):
>
> - create a disk image, call it hda-1.img
> - boot and install linux on it, shutdown
> - copy to hda-2.img
> - boot it (with new MAC) and change IP, hostname, little things, shutdown
> - boot both with the same bridge, check that network works between them
> - create a new disk image, call id hdb-shr.img
> - boot both VMs, sharing hdb-shr.img:
>
> qemu -hda=hda-1.img -hdb-shr.img
> qemu -hda=hda-2.img -hdb-shr.img
>
> - try to setup a cluster filesystem on hdb
>
> it almost worked... but some writes didn't propagate to the other
> until some extra writes to hdb; so i guessed that each qemu instance
> had some caching on file I/O
>
> hopefully, it would now work with "-cache=off", don't you think?
>   
>

cache=off is not necessary.  However you need raw format images (not 
qcow/vmdk/etc).


-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2008-01-07 18:32 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-01-04  7:11 [PATCH] qemu: use statically allocate 512 byte buffer in the stack for sector in bdrv_commit Carlo Marcelo Arenas Belon
2008-01-07  9:27 ` Avi Kivity
     [not found]   ` <4781F08E.8060407-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
2008-01-07 10:22     ` Laurent Vivier
2008-01-07 10:47       ` Avi Kivity
     [not found]         ` <4782034C.4000805-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
2008-01-07 12:08           ` Laurent Vivier
2008-01-07 15:16           ` Laurent Vivier
2008-01-07 15:34             ` Javier Guerra
     [not found]               ` <90eb1dc70801070734l2062cac6r7a7bed1d6d3d2c0c-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-01-07 15:45                 ` Laurent Vivier
2008-01-07 16:03                   ` Javier Guerra
     [not found]                     ` <90eb1dc70801070803w1a863acs75677e707446f79a-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-01-07 16:30                       ` Laurent Vivier
2008-01-07 16:42                         ` Javier Guerra
     [not found]                           ` <90eb1dc70801070842g585cd92dr8a8a383e2f3274df-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2008-01-07 17:05                             ` Laurent Vivier
2008-01-07 18:32                       ` Avi Kivity
2008-01-07 18:29                 ` Avi Kivity
2008-01-07 18:27             ` Avi Kivity

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox