qemu: use statically allocate 512 byte buffer in the stack for sector in bdrv_commit
PATCH 1/1 qemu: use statically allocate 512 byte buffer in the stack for sector in bdrv_commit
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
--- qemu/block.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
qemu/block.c +1 -1
@@ -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 ;
Signature · 7 lines
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/
Quoted 10 lines · Carlo Marcelo Arenas Belon
revert a merge conflict from
075da586c92f09bd9a7401f1e80d72fde27c173that 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 ofbdrv_read' from incompatible pointer type block.c:484: warning: passing arg 3 ofbdrv_write' from incompatible pointer typeSigned-off-by Carlo Marcelo Arenas Belon <[email protected]>
Doesn't the cache=off option warrant an allocation here to ensure alignment (or perhaps a 1K stack buffer with runtime adjustment)?
Signature · 7 lines
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/
A message that is not in the archive (sent off-list or not yet received): 4781F08E.8060407-atKUWr5tajBWk0Htik3J/[email protected]
Quoted 9 lines · Avi Kivity
Quoted 8 lines · Carlo Marcelo Arenas Belon
revert a merge conflict from
075da586c92f09bd9a7401f1e80d72fde27c173that 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 ofbdrv_read' from incompatible pointer type block.c:484: warning: passing arg 3 ofbdrv_write' from incompatible pointer type
Quoted 5 lines
Quoted 2 lines
Signed-off-by Carlo Marcelo Arenas Belon <[email protected]>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) :
qemu/block.c +8 -1
@@ -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);
Signature · 3 lines
----------------- Laurent.Vivier-6ktuUTfB/[email protected] ------------------ "La perfection est atteinte non quand il ne reste rien à ajouter mais quand il ne reste rien à enlever." Saint Exupéry
-------------------------------------------------------------------------
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 [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Quoted 15 lines · Laurent Vivier
Quoted 13 lines · Avi Kivity
Quoted 11 lines · Carlo Marcelo Arenas Belon
revert a merge conflict from
075da586c92f09bd9a7401f1e80d72fde27c173that 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 ofbdrv_read' from incompatible pointer type block.c:484: warning: passing arg 3 ofbdrv_write' from incompatible pointer typeSigned-off-by Carlo Marcelo Arenas Belon <[email protected]>
Quoted 7 lines
Quoted 3 lines
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.
Signature · 11 lines
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 [email protected] https://lists.sourceforge.net/lists/listinfo/kvm-devel
A message that is not in the archive (sent off-list or not yet received): 4782034C.4000805-atKUWr5tajBWk0Htik3J/[email protected]
Quoted 20 lines · Avi Kivity
Quoted 19 lines · Laurent Vivier
Quoted 16 lines · Avi Kivity
Quoted 11 lines · Carlo Marcelo Arenas Belon
revert a merge conflict from
075da586c92f09bd9a7401f1e80d72fde27c173that 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 ofbdrv_read' from incompatible pointer type block.c:484: warning: passing arg 3 ofbdrv_write' from incompatible pointer typeSigned-off-by Carlo Marcelo Arenas Belon <[email protected]>Doesn't the cache=off option warrant an allocation here to ensure alignment (or perhaps a 1K stack buffer with runtime adjustment)?
Quoted 5 lines
Quoted 3 lines
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
Signature · 3 lines
----------------- Laurent.Vivier-6ktuUTfB/[email protected] ------------------ "La perfection est atteinte non quand il ne reste rien à ajouter mais quand il ne reste rien à enlever." Saint Exupéry
-------------------------------------------------------------------------
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 [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Quoted 20 lines · Avi Kivity
Quoted 19 lines · Laurent Vivier
Quoted 16 lines · Avi Kivity
Quoted 11 lines · Carlo Marcelo Arenas Belon
revert a merge conflict from
075da586c92f09bd9a7401f1e80d72fde27c173that 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 ofbdrv_read' from incompatible pointer type block.c:484: warning: passing arg 3 ofbdrv_write' from incompatible pointer typeSigned-off-by Carlo Marcelo Arenas Belon <[email protected]>Doesn't the cache=off option warrant an allocation here to ensure alignment (or perhaps a 1K stack buffer with runtime adjustment)?
Quoted 5 lines
Quoted 3 lines
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
Signature · 3 lines
----------------- Laurent.Vivier-6ktuUTfB/[email protected] ------------------ "La perfection est atteinte non quand il ne reste rien à ajouter mais quand il ne reste rien à enlever." Saint Exupéry
-------------------------------------------------------------------------
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 [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Quoted 2 lines · Laurent Vivier
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
Signature · 6 lines
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/
A message that is not in the archive (sent off-list or not yet received): 90eb1dc70801070734l2062cac6r7a7bed1d6d3d2c0c-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Quoted 6 lines · Javier Guerra
Quoted 2 lines · Laurent Vivier
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.
Quoted 3 lines
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.
Quoted 1 line
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
Signature · 3 lines
----------------- Laurent.Vivier-6ktuUTfB/[email protected] ------------------ "La perfection est atteinte non quand il ne reste rien à ajouter mais quand il ne reste rien à enlever." Saint Exupéry
-------------------------------------------------------------------------
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 [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Quoted 10 lines · Laurent Vivier
"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.Quoted 3 lines
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?
Quoted 3 lines
Quoted 1 line
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?
Signature · 6 lines
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/
A message that is not in the archive (sent off-list or not yet received): 90eb1dc70801070803w1a863acs75677e707446f79a-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Quoted 26 lines · Javier Guerra
Quoted 10 lines · Laurent Vivier
"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.Quoted 3 lines
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
Quoted 7 lines
- 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" ?
Quoted 9 lines
Quoted 3 lines
Quoted 1 line
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
Signature · 3 lines
----------------- Laurent.Vivier-6ktuUTfB/[email protected] ------------------ "La perfection est atteinte non quand il ne reste rien à ajouter mais quand il ne reste rien à enlever." Saint Exupéry
-------------------------------------------------------------------------
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 [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Quoted 7 lines · Laurent Vivier
Quoted 1 line · Javier Guerra
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)
Quoted 4 lines
There are also some caches at qemu level to emulate DMA, for instance in
hw/ide.cit isMAX_MULT_SECTORS(16) which is 8 kB buffer, perhaps your problem is here but "cache=off" doesn't remove this.
Did you try to changeMAX_MULT_SECTORSto 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
Quoted 1 line
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.
Signature · 9 lines
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 [email protected] https://lists.sourceforge.net/lists/listinfo/kvm-devel
A message that is not in the archive (sent off-list or not yet received): 90eb1dc70801070842g585cd92dr8a8a383e2f3274df-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
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
Quoted 32 lines · Javier Guerra
Quoted 7 lines · Laurent Vivier
Quoted 1 line · Javier Guerra
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)
Quoted 4 lines
There are also some caches at qemu level to emulate DMA, for instance in
hw/ide.cit isMAX_MULT_SECTORS(16) which is 8 kB buffer, perhaps your problem is here but "cache=off" doesn't remove this.
Did you try to changeMAX_MULT_SECTORSto 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
Quoted 1 line
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.
Signature · 3 lines
----------------- Laurent.Vivier-6ktuUTfB/[email protected] ------------------ "La perfection est atteinte non quand il ne reste rien à ajouter mais quand il ne reste rien à enlever." Saint Exupéry
-------------------------------------------------------------------------
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
_______________________________________________
kvm-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Quoted 14 lines · Javier Guerra
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
Quoted 9 lines
- 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).
Signature · 8 lines
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
Quoted 6 lines · Javier Guerra
Quoted 3 lines · Laurent Vivier
What I'm wondering now is: is it really useful to have "cache=off" and "snapshot=on" at the same time ?
Quoted 7 lines
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).
Signature · 8 lines
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
Quoted 3 lines · Laurent Vivier
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.
Quoted 3 lines
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.
Signature · 8 lines
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