Re: [Qemu-devel] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified

2011-06-18 Thread Stefan Weil

Am 18.06.2011 07:13, schrieb Roy Tam:

This patch fix conflicting types for 'INT32' in basetsd.h in including
qemu-common.h first.


Sign-off-by: Roy Tam
--
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 87fdf35..1591df0 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -28,6 +28,8 @@

  #include "config-host.h"

+#include "qemu-common.h"
+
  #ifdef CONFIG_VNC_PNG
  #include
  #endif
@@ -36,8 +38,6 @@
  #include
  #endif

-#include "qemu-common.h"
-
  #include "bswap.h"
  #include "qint.h"
  #include "vnc.h"


Acked-by: Stefan Weil 

The conflicting declaration is in jmorecfg.h which is included from 
jpeglib.h.






Re: [Qemu-devel] Image streaming and live block copy

2011-06-18 Thread Stefan Hajnoczi
On Fri, Jun 17, 2011 at 1:31 PM, Marcelo Tosatti  wrote:
> On Thu, Jun 16, 2011 at 04:30:18PM +0100, Stefan Hajnoczi wrote:
>> On Thu, Jun 16, 2011 at 11:52:43AM -0300, Marcelo Tosatti wrote:
>> This approach does not use the backing file feature?
>>
>> > blkstream block driver:
>> >
>> > - Maintain in memory whether given block is allocated in local image,
>> > if not, read from remote, write to local. Set block as local.
>> > Local and remote simply two block drivers from image streaming driver
>> > POV.
>> > - Once all blocks are local, notify mgmt so it can switch to local
>> > copy.
>> > - Writes are mirrored to source and destination, minding guest writes
>> > over copy writes.
>>
>> We open the remote file read-only for image streaming and do not want to
>> mirror writes.
>
> Why not? Is there any disadvantage of mirroring writes?

Think of the use case with a Fedora master image over NFS.  You want a
local clone of that master image and use the stream command to copy
the data from the master image into the local clone.

You cannot modify that master image because other VMs are using it too
and/or you want to be able to clone new VMs from it in the future.

Stefan



Re: [Qemu-devel] Image streaming and live block copy

2011-06-18 Thread Stefan Hajnoczi
On Sat, Jun 18, 2011 at 10:15 AM, Stefan Hajnoczi  wrote:
> On Fri, Jun 17, 2011 at 1:31 PM, Marcelo Tosatti  wrote:
>> On Thu, Jun 16, 2011 at 04:30:18PM +0100, Stefan Hajnoczi wrote:
>>> On Thu, Jun 16, 2011 at 11:52:43AM -0300, Marcelo Tosatti wrote:
>>> This approach does not use the backing file feature?
>>>
>>> > blkstream block driver:
>>> >
>>> > - Maintain in memory whether given block is allocated in local image,
>>> > if not, read from remote, write to local. Set block as local.
>>> > Local and remote simply two block drivers from image streaming driver
>>> > POV.
>>> > - Once all blocks are local, notify mgmt so it can switch to local
>>> > copy.
>>> > - Writes are mirrored to source and destination, minding guest writes
>>> > over copy writes.
>>>
>>> We open the remote file read-only for image streaming and do not want to
>>> mirror writes.
>>
>> Why not? Is there any disadvantage of mirroring writes?
>
> Think of the use case with a Fedora master image over NFS.  You want a
> local clone of that master image and use the stream command to copy
> the data from the master image into the local clone.
>
> You cannot modify that master image because other VMs are using it too
> and/or you want to be able to clone new VMs from it in the future.

BTW the workaround is to create two local images:
1. Local clone with master image as a backing file.  This is the live
block copy source image.
2. Local image without a backing file.  This is the live block copy
destination image.

But this is not very elegant.  Writes get mirrored so that crash recovery works.

Compare that to image streaming, which works across crash and doesn't
duplicate I/O.

Stefan



Re: [Qemu-devel] [PATCH 2/9] target-ppc: Handle memory-forced I/O controller access

2011-06-18 Thread Andreas Färber

Am 18.06.2011 um 00:59 schrieb Alexander Graf:


On 17.06.2011, at 21:34, Andreas Färber wrote:


Am 17.06.2011 um 16:43 schrieb Alexander Graf:


From: Hervé Poussineau 

On at least the PowerPC 601, a direct-store (T=1) with bus unit ID  
0x07F
is special-cased as memory-forced I/O controller access. It is  
supposed
to be checked immediately if T=1, bypassing all protection  
mechanisms

and acting cache-inhibited and global.

Signed-off-by: Hervé Poussineau 

Simplified by avoiding reindentation. Added explanatory comments.

Cc: Alexander Graf 
Signed-off-by: Andreas Färber 
Signed-off-by: Alexander Graf 


Feel free to remove the Cc: after adding a corresponding Sob or  
similar line.


Hrm - I usually just use git am -s for the sob line. If you find the  
CC irritating, please just pass it to git send-email --cc and omit  
it from the patch. It's more or less unuseful information anyways :)


Ah, didn't know that option. I always used git am, followed by git  
commit --amend -s, at which point one can edit the commit message.


--cc becomes unhandy in a series of ~35, when not all patches concern  
everyone and to avoid forgetting CCs.


Andreas


Re: [Qemu-devel] [PATCH 2/9] target-ppc: Handle memory-forced I/O controller access

2011-06-18 Thread Alexander Graf

On 18.06.2011, at 13:15, Andreas Färber wrote:

> Am 18.06.2011 um 00:59 schrieb Alexander Graf:
> 
>> On 17.06.2011, at 21:34, Andreas Färber wrote:
>> 
>>> Am 17.06.2011 um 16:43 schrieb Alexander Graf:
>>> 
 From: Hervé Poussineau 
 
 On at least the PowerPC 601, a direct-store (T=1) with bus unit ID 0x07F
 is special-cased as memory-forced I/O controller access. It is supposed
 to be checked immediately if T=1, bypassing all protection mechanisms
 and acting cache-inhibited and global.
 
 Signed-off-by: Hervé Poussineau 
 
 Simplified by avoiding reindentation. Added explanatory comments.
 
 Cc: Alexander Graf 
 Signed-off-by: Andreas Färber 
 Signed-off-by: Alexander Graf 
>>> 
>>> Feel free to remove the Cc: after adding a corresponding Sob or similar 
>>> line.
>> 
>> Hrm - I usually just use git am -s for the sob line. If you find the CC 
>> irritating, please just pass it to git send-email --cc and omit it from the 
>> patch. It's more or less unuseful information anyways :)
> 
> Ah, didn't know that option. I always used git am, followed by git commit 
> --amend -s, at which point one can edit the commit message.

I see - that's pretty annoying when it comes to applying patch sets though, no? 
:)

> --cc becomes unhandy in a series of ~35, when not all patches concern 
> everyone and to avoid forgetting CCs.

That's where taste differs :). Avi usually even drops the automatic CCs and 
manually adds --cc for the whole patch set to people it concerns. I somewhat 
sympathize with this approach. Who wants patch 16/35 when he doesn't get all 
the others, especially not 00/35? I certainly prefer to get the full set in my 
inbox instead of just the patches that relate explicitly to me.


Alex




Re: [Qemu-devel] [PATCH v5 3/5] guest agent: add guest agent RPCs/commands

2011-06-18 Thread Luiz Capitulino
On Fri, 17 Jun 2011 23:38:16 -0300
Luiz Capitulino  wrote:

> On Fri, 17 Jun 2011 15:19:56 -0500
> Michael Roth  wrote:
> 
> > On 06/16/2011 01:52 PM, Luiz Capitulino wrote:
> > > On Tue, 14 Jun 2011 15:06:23 -0500
> > > Michael Roth  wrote:
> > >
> > >> This adds the initial set of QMP/QAPI commands provided by the guest
> > >> agent:
> > >>
> > >> guest-sync
> > >> guest-ping
> > >> guest-info
> > >> guest-shutdown
> > >> guest-file-open
> > >> guest-file-read
> > >> guest-file-write
> > >> guest-file-seek
> > >> guest-file-close
> > >> guest-fsfreeze-freeze
> > >> guest-fsfreeze-thaw
> > >> guest-fsfreeze-status
> > >>
> > >> The input/output specification for these commands are documented in the
> > >> schema.
> > >>
> > >> Signed-off-by: Michael Roth
> > >> ---
> > >>   qerror.c   |4 +
> > >>   qerror.h   |3 +
> > >>   qga/guest-agent-commands.c |  522 
> > >> 
> > >>   qga/guest-agent-core.h |1 +
> > >>   4 files changed, 530 insertions(+), 0 deletions(-)
> > >>   create mode 100644 qga/guest-agent-commands.c
> > >>
> > >> diff --git a/qerror.c b/qerror.c
> > >> index d7fcd93..24f0c48 100644
> > >> --- a/qerror.c
> > >> +++ b/qerror.c
> > >> @@ -213,6 +213,10 @@ static const QErrorStringTable qerror_table[] = {
> > >>   .error_fmt = QERR_VNC_SERVER_FAILED,
> > >>   .desc  = "Could not start VNC server on %(target)",
> > >>   },
> > >> +{
> > >> +.error_fmt = QERR_QGA_LOGGING_FAILED,
> > >> +.desc  = "failed to write log statement due to logging 
> > >> being disabled",
> > >> +},
> > >>   {}
> > >>   };
> > >>
> > >> diff --git a/qerror.h b/qerror.h
> > >> index 7a89a50..bf3d5a9 100644
> > >> --- a/qerror.h
> > >> +++ b/qerror.h
> > >> @@ -184,4 +184,7 @@ QError *qobject_to_qerror(const QObject *obj);
> > >>   #define QERR_FEATURE_DISABLED \
> > >>   "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }"
> > >>
> > >> +#define QERR_QGA_LOGGING_FAILED \
> > >> +"{ 'class': 'QgaLoggingFailed', 'data': {} }"
> > >> +
> > >>   #endif /* QERROR_H */
> > >> diff --git a/qga/guest-agent-commands.c b/qga/guest-agent-commands.c
> > >> new file mode 100644
> > >> index 000..6f9886a
> > >> --- /dev/null
> > >> +++ b/qga/guest-agent-commands.c
> > >> @@ -0,0 +1,522 @@
> > >> +/*
> > >> + * QEMU Guest Agent commands
> > >> + *
> > >> + * Copyright IBM Corp. 2011
> > >> + *
> > >> + * Authors:
> > >> + *  Michael Roth
> > >> + *
> > >> + * This work is licensed under the terms of the GNU GPL, version 2 or 
> > >> later.
> > >> + * See the COPYING file in the top-level directory.
> > >> + */
> > >> +
> > >> +#include
> > >> +#include
> > >> +#include
> > >> +#include
> > >> +#include
> > >> +#include "qga/guest-agent-core.h"
> > >> +#include "qga-qmp-commands.h"
> > >> +#include "qerror.h"
> > >> +
> > >> +static GAState *ga_state;
> > >> +
> > >> +static bool logging_enabled(void)
> > >> +{
> > >> +return ga_logging_enabled(ga_state);
> > >> +}
> > >> +
> > >> +static void disable_logging(void)
> > >> +{
> > >> +ga_disable_logging(ga_state);
> > >> +}
> > >> +
> > >> +static void enable_logging(void)
> > >> +{
> > >> +ga_enable_logging(ga_state);
> > >> +}
> > >> +
> > >> +/* Note: in some situations, like with the fsfreeze, logging may be
> > >> + * temporarilly disabled. if it is necessary that a command be able
> > >> + * to log for accounting purposes, check logging_enabled() beforehand,
> > >> + * and use the QERR_QGA_LOGGING_DISABLED to generate an error
> > >> + */
> > >> +static void slog(const char *fmt, ...)
> > >> +{
> > >> +va_list ap;
> > >> +
> > >> +va_start(ap, fmt);
> > >> +g_logv("syslog", G_LOG_LEVEL_INFO, fmt, ap);
> > >> +va_end(ap);
> > >> +}
> > >> +
> > >> +int64_t qmp_guest_sync(int64_t id, Error **errp)
> > >> +{
> > >> +return id;
> > >> +}
> > >> +
> > >> +void qmp_guest_ping(Error **err)
> > >> +{
> > >> +slog("guest-ping called");
> > >> +}
> > >> +
> > >> +struct GuestAgentInfo *qmp_guest_info(Error **err)
> > >> +{
> > >> +GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo));
> > >> +
> > >> +info->version = g_strdup(QGA_VERSION);
> > >> +
> > >> +return info;
> > >> +}
> > >> +
> > >> +void qmp_guest_shutdown(const char *shutdown_mode, Error **err)
> > >
> > > I'd call the argument just 'mode'.
> > >
> > >> +{
> > >> +int ret;
> > >> +const char *shutdown_flag;
> > >> +
> > >> +if (!logging_enabled()) {
> > >> +error_set(err, QERR_QGA_LOGGING_FAILED);
> > >> +return;
> > >> +}
> > >> +
> > >> +slog("guest-shutdown called, shutdown_mode: %s", shutdown_mode);
> > >> +if (strcmp(shutdown_mode, "halt") == 0) {
> > >> +shutdown_flag = "-H";
> > >> +} else if (strcmp(shutdown_mode, "powerdown") == 0) {
> > >> +shutdown_flag = "-P";
> > >> +} else if (strcmp(shutdown_mode, "reboot") == 0) {
> > >> +shutdown_

Re: [Qemu-devel] [PATCH 04/12] VMDK: separate vmdk_open by format version

2011-06-18 Thread Stefan Hajnoczi
On Sat, Jun 4, 2011 at 1:41 AM, Fam Zheng  wrote:
> +static int vmdk_open_vmdk4(BlockDriverState *bs, int flags)
> +{
> +    uint32_t magic;
> +    VMDK4Header header;
> +    BDRVVmdkState *s = bs->opaque;
> +    VmdkExtent *extent;
> +
> +    s->extents = qemu_mallocz(sizeof(VmdkExtent));
> +    s->num_extents = 1;
> +    if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header))
> +        != sizeof(header)) {
> +        goto fail;
> +    }
> +    extent = s->extents;
> +    extent->file = bs->file;
> +    extent->sectors = le64_to_cpu(header.capacity);
> +    extent->cluster_sectors = le64_to_cpu(header.granularity);
> +    extent->l2_size = le32_to_cpu(header.num_gtes_per_gte);
> +    extent->l1_entry_sectors = extent->l2_size * extent->cluster_sectors;
> +    if (extent->l1_entry_sectors <= 0) {
> +        goto fail;
> +    }
> +    extent->l1_size = (extent->sectors + extent->l1_entry_sectors - 1)
> +        / extent->l1_entry_sectors;
> +    extent->l1_table_offset = le64_to_cpu(header.rgd_offset) << 9;
> +    extent->l1_backup_table_offset = le64_to_cpu(header.gd_offset) << 9;
> +
> +    // try to open parent images, if exist
> +    if (vmdk_parent_open(bs) != 0) {
> +        goto fail;
> +    }
> +    // write the CID once after the image creation

This comment is from vmdk_create() and can be removed?

> +    extent->parent_cid = vmdk_read_cid(bs,1);
> +    bs->total_sectors = extent->sectors;
> +    if (vmdk_init_tables(bs, extent)) {
> +        goto fail;
>     }
> +    return 0;
> + fail:
> +    qemu_free(s->extents);
>     return -1;
>  }
>
> +static int vmdk_open(BlockDriverState *bs, int flags)
> +{
> +    uint32_t magic;
> +
> +    if (bdrv_pread(bs->file, 0, &magic, sizeof(magic)) != sizeof(magic)) {
> +        return -1;
> +    }
> +
> +    magic = be32_to_cpu(magic);
> +    if (magic == VMDK3_MAGIC) {
> +        return vmdk_open_vmdk3(bs, flags);
> +    } else if (magic == VMDK4_MAGIC) {
> +        return vmdk_open_vmdk4(bs, flags);
> +    } else {
> +        return -1;
> +    }

This is a good opportunity to use -errno instead of -1.  If you look
at qcow2 or qed they return appropriate error numbers and not just -1.
 (Also remember -1 is -EPERM so callers that interpret the return
value as an error will print junk error messages if -1 is returned.)

Stefan



Re: [Qemu-devel] [PATCH 1/3] kvm: ppc: booke206: use MMU API

2011-06-18 Thread Richard Henderson
On 06/17/2011 04:28 PM, Alexander Graf wrote:
>> > +struct kvm_book3e_206_tlb_params params = {};
> Hrm - I'm not familiar with that initialization. What exactly does it
> do? Set the struct contents to 0? Is this properly standardized?

Yes and yes.


r~



Re: [Qemu-devel] [PATCH v5 1/5] Add hard build dependency on glib

2011-06-18 Thread Andreas Färber

Am 12.06.2011 um 22:46 schrieb Stefan Hajnoczi:


From: Anthony Liguori 

GLib is an extremely common library that has a portable thread  
implementation

along with tons of other goodies.

GLib and GObject have a fantastic amount of infrastructure we can  
leverage in

QEMU including an object oriented programming infrastructure.


GObject still doesn't build on Darwin/ppc64:

  CC gboxed.lo
../../glib-2.28.8/gobject/gboxed.c: In function 'g_closure_get_type':
../../glib-2.28.8/gobject/gboxed.c:120: warning: union cannot be made  
transparent
../../glib-2.28.8/gobject/gboxed.c:120: warning: union cannot be made  
transparent
../../glib-2.28.8/gobject/gboxed.c:120: error: incompatible type for  
argument 2 of '_g_register_boxed'
../../glib-2.28.8/gobject/gboxed.c:120: error: incompatible type for  
argument 3 of '_g_register_boxed'

../../glib-2.28.8/gobject/gboxed.c: In function 'g_value_get_type':
../../glib-2.28.8/gobject/gboxed.c:121: warning: union cannot be made  
transparent
../../glib-2.28.8/gobject/gboxed.c:121: warning: union cannot be made  
transparent
../../glib-2.28.8/gobject/gboxed.c:121: error: incompatible type for  
argument 2 of '_g_register_boxed'
../../glib-2.28.8/gobject/gboxed.c:121: error: incompatible type for  
argument 3 of '_g_register_boxed'
../../glib-2.28.8/gobject/gboxed.c: In function  
'g_value_array_get_type':
../../glib-2.28.8/gobject/gboxed.c:122: warning: union cannot be made  
transparent
../../glib-2.28.8/gobject/gboxed.c:122: warning: union cannot be made  
transparent
../../glib-2.28.8/gobject/gboxed.c:122: error: incompatible type for  
argument 2 of '_g_register_boxed'
../../glib-2.28.8/gobject/gboxed.c:122: error: incompatible type for  
argument 3 of '_g_register_boxed'

../../glib-2.28.8/gobject/gboxed.c: In function 'g_date_get_type':
../../glib-2.28.8/gobject/gboxed.c:123: warning: union cannot be made  
transparent
../../glib-2.28.8/gobject/gboxed.c:123: warning: union cannot be made  
transparent
../../glib-2.28.8/gobject/gboxed.c:123: error: incompatible type for  
argument 2 of '_g_register_boxed'
../../glib-2.28.8/gobject/gboxed.c:123: error: incompatible type for  
argument 3 of '_g_register_boxed'

../../glib-2.28.8/gobject/gboxed.c: In function 'g_gstring_get_type':
../../glib-2.28.8/gobject/gboxed.c:125: warning: union cannot be made  
transparent
../../glib-2.28.8/gobject/gboxed.c:125: warning: union cannot be made  
transparent
../../glib-2.28.8/gobject/gboxed.c:125: error: incompatible type for  
argument 2 of '_g_register_boxed'
../../glib-2.28.8/gobject/gboxed.c:125: error: incompatible type for  
argument 3 of '_g_register_boxed'

../../glib-2.28.8/gobject/gboxed.c: In function 'g_hash_table_get_type':
../../glib-2.28.8/gobject/gboxed.c:126: warning: union cannot be made  
transparent
../../glib-2.28.8/gobject/gboxed.c:126: warning: union cannot be made  
transparent
../../glib-2.28.8/gobject/gboxed.c:126: error: incompatible type for  
argument 2 of '_g_register_boxed'
../../glib-2.28.8/gobject/gboxed.c:126: error: incompatible type for  
argument 3 of '_g_register_boxed'

../../glib-2.28.8/gobject/gboxed.c: In function 'g_array_get_type':
../../glib-2.28.8/gobject/gboxed.c:127: warning: union cannot be made  
transparent
../../glib-2.28.8/gobject/gboxed.c:127: warning: union cannot be made  
transparent
../../glib-2.28.8/gobject/gboxed.c:127: error: incompatible type for  
argument 2 of '_g_register_boxed'
../../glib-2.28.8/gobject/gboxed.c:127: error: incompatible type for  
argument 3 of '_g_register_boxed'

../../glib-2.28.8/gobject/gboxed.c: In function 'g_ptr_array_get_type':
../../glib-2.28.8/gobject/gboxed.c:128: warning: union cannot be made  
transparent
../../glib-2.28.8/gobject/gboxed.c:128: warning: union cannot be made  
transparent
../../glib-2.28.8/gobject/gboxed.c:128: error: incompatible type for  
argument 2 of '_g_register_boxed'
../../glib-2.28.8/gobject/gboxed.c:128: error: incompatible type for  
argument 3 of '_g_register_boxed'

../../glib-2.28.8/gobject/gboxed.c: In function 'g_byte_array_get_type':
../../glib-2.28.8/gobject/gboxed.c:129: warning: union cannot be made  
transparent
../../glib-2.28.8/gobject/gboxed.c:129: warning: union cannot be made  
transparent
../../glib-2.28.8/gobject/gboxed.c:129: error: incompatible type for  
argument 2 of '_g_register_boxed'
../../glib-2.28.8/gobject/gboxed.c:129: error: incompatible type for  
argument 3 of '_g_register_boxed'

../../glib-2.28.8/gobject/gboxed.c: In function 'g_regex_get_type':
../../glib-2.28.8/gobject/gboxed.c:132: warning: union cannot be made  
transparent
../../glib-2.28.8/gobject/gboxed.c:132: warning: union cannot be made  
transparent
../../glib-2.28.8/gobject/gboxed.c:132: error: incompatible type for  
argument 2 of '_g_register_boxed'
../../glib-2.28.8/gobject/gboxed.c:132: error: incompatible type for  
argument 3 of '_g_register_boxed'
../../glib-2.28.8/gobject/gboxed.c: In function  
'g_variant_type_get_gtype':
../../glib-2.28.8/gobject/g

Re: [Qemu-devel] [PATCH 1/3] kvm: ppc: booke206: use MMU API

2011-06-18 Thread Alexander Graf

On 18.06.2011, at 18:13, Richard Henderson wrote:

> On 06/17/2011 04:28 PM, Alexander Graf wrote:
 +struct kvm_book3e_206_tlb_params params = {};
>> Hrm - I'm not familiar with that initialization. What exactly does it
>> do? Set the struct contents to 0? Is this properly standardized?
> 
> Yes and yes.

Ah, very nice. I wonder why I don't see it used more in code then :). Seems to 
be very handy to not clutter code with memset(0)s.


Alex




Re: [Qemu-devel] [PATCH 06/12] VMDK: vmdk_open for mono flat

2011-06-18 Thread Stefan Hajnoczi
On Sat, Jun 4, 2011 at 1:42 AM, Fam Zheng  wrote:
> Vmdk_open for mono flat image.
>
> Signed-off-by: Fam Zheng 
> ---
>  block/vmdk.c |  134 
> +++---
>  1 files changed, 128 insertions(+), 6 deletions(-)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index b02a7b7..f1233cf 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -355,24 +355,110 @@ static int vmdk_parent_open(BlockDriverState *bs)
>     char desc[DESC_SIZE];
>     BDRVVmdkState *s = bs->opaque;
>
> -    if (bdrv_pread(bs->file, s->desc_offset, desc, DESC_SIZE) != DESC_SIZE)
> +    if (bdrv_pread(bs->file, s->desc_offset, desc, DESC_SIZE) != DESC_SIZE) {
>         return -1;
> +    }
>
>     if ((p_name = strstr(desc,"parentFileNameHint")) != NULL) {

Same strstr(3) issue here in the existing vmdk code.

I suggest increasing the size by one, desc[DESC_SIZE + 1], and setting
the last char to '\0' so that strstr(3) never runs off the end of the
desc[] buffer.

>         char *end_name;
>
>         p_name += sizeof("parentFileNameHint") + 1;
> -        if ((end_name = strchr(p_name,'\"')) == NULL)
> +        if ((end_name = strchr(p_name,'\"')) == NULL) {
>             return -1;
> -        if ((end_name - p_name) > sizeof (bs->backing_file) - 1)
> +        }
> +        if ((end_name - p_name) > sizeof (bs->backing_file) - 1) {
>             return -1;
> -
> +        }
>         pstrcpy(bs->backing_file, end_name - p_name + 1, p_name);
>     }
> -
>     return 0;
>  }
>
> +/* find an option value out of descriptor file */
> +static int vmdk_parse_description(const char *desc, const char *opt_name,
> +        char *buf, int buf_size)
> +{
> +    char *opt_pos = strstr(desc, opt_name);
> +    int r;
> +    if (!opt_pos) {
> +        return -1;
> +    }
> +    opt_pos += strlen(opt_name) + 2;

Couldn't the opt_name match at the end of the descriptor?  This would
jump beyond the NUL terminator!

> +    r = sscanf(opt_pos, "%[^\"]s", buf);
> +    assert(r <= buf_size);

You cannot use assert for input validation.  File format parsing needs
to be bulletproof, there can be no crashes or asserts.

> +    return r <= 0;
> +}
> +
> +
> +static int vmdk_parse_extents(const char *desc, VmdkExtent extents[],
> +        const char *desc_file_path)
> +{
> +    int ret = 0;
> +    int r;
> +    char access[11];
> +    char type[11];
> +    char fname[512];
> +    VmdkExtent *extent;
> +    const char *p = desc;
> +    int64_t sectors = 0;
> +
> +    while (*p) {
> +        if (strncmp(p, "RW", strlen("RW")) == 0) {
> +            /* parse extent line:
> +             * RW [size in sectors] FLAT "file-name.vmdk" OFFSET
> +             * or
> +             * RW [size in sectors] SPARSE "file-name.vmdk"
> +             */
> +
> +            sscanf(p, "%10s %lld %10s \"%[^\"]512s\"",
> +                    access, §ors, type, fname);

You can combine the "RW" strncmp into the format string and then check
the sscanf(3) return value to see if the line matched.  Then you can
also avoid the big if statement.

> +            if (!(strlen(access) && sectors && strlen(type) &&
> strlen(fname))) {
> +                goto cont;
> +            }
> +            if (strcmp(type, "FLAT") && strcmp(type, "SPARSE")) {
> +                goto cont;
> +            }
> +            if (strcmp(access, "RW")) {
> +                goto cont;
> +            }
> +            ret++;
> +            if (!extents) {
> +                goto cont;
> +            }
> +
> +            /* save to extents array */
> +            if (!strcmp(type, "FLAT")) {
> +                /* FLAT extent */
> +                char extent_path[1024];

There is a constant for this: PATH_MAX

> +                path_combine(extent_path, sizeof(extent_path),
> +                        desc_file_path, fname);
> +                extent = &extents[ret - 1];
> +                extent->flat = true;
> +                extent->sectors = sectors;
> +                extent->cluster_sectors = sectors;
> +                extent->file = bdrv_new("");
> +                if (!extent->file) {
> +                    return -1;
> +                }
> +                r = bdrv_open(extent->file, extent_path,
> +                        BDRV_O_RDWR | BDRV_O_NO_BACKING, NULL);
> +                if (r) {
> +                    return -1;
> +                }
> +            } else {
> +                /* SPARSE extent, should not be here */
> +                fprintf(stderr, "VMDK: Not supported extent type.\n");

For troubleshooting purposes it would be useful to print the
unsupported extent type.

> +                return -1;
> +            }
> +        }
> +cont:
> +        /* move to next line */
> +        while (*p && *p != '\n') p++;
> +        p++;
> +    }
> +    return ret;
> +}
> +
>  static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent)
>  {
>     int l1_size, i;
> @@ -496,6 +582,42 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, int 
> flags)
>     re

Re: [Qemu-devel] [PATCH 08/12] VMDK: vmdk_close for extents

2011-06-18 Thread Stefan Hajnoczi
On Sat, Jun 4, 2011 at 1:42 AM, Fam Zheng  wrote:
> Signed-off-by: Fam Zheng 
> ---
>  block/vmdk.c |    9 +++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 1d74b62..bbab68a 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1086,10 +1086,15 @@ exit:
>
>  static void vmdk_close(BlockDriverState *bs)
>  {
> +    int i;
>     BDRVVmdkState *s = bs->opaque;
>
> -    qemu_free(s->extents[0].l1_table);
> -    qemu_free(s->extents[0].l2_cache);
> +    for (i = 0; i < s->num_extents; i++) {
> +        qemu_free(s->extents[i].l1_table);
> +        qemu_free(s->extents[i].l2_cache);
> +        qemu_free(s->extents[i].l1_backup_table);
> +    }
> +    qemu_free(s->extents);
>  }

Should this patch be moved/merged earlier in the series to prevent
leaks?  (Each commit in the series should build and execute correctly.
 There should be no intermediate leaks, crashes, or other problems.)

Stefan



Re: [Qemu-devel] [PATCH 07/12] VMDK: vmdk_flush for extents

2011-06-18 Thread Stefan Hajnoczi
On Sat, Jun 4, 2011 at 1:42 AM, Fam Zheng  wrote:
> Vmdk flush in extent array style.
>
>
> Signed-off-by: Fam Zheng 
> ---
>  block/vmdk.c |    9 -
>  1 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index f1233cf..1d74b62 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1094,7 +1094,14 @@ static void vmdk_close(BlockDriverState *bs)
>
>  static int vmdk_flush(BlockDriverState *bs)
>  {
> -    return bdrv_flush(bs->file);
> +    int i, ret;
> +    BDRVVmdkState *s = bs->opaque;
> +
> +    ret = bdrv_flush(bs->file);
> +    for (i = 0; i < s->num_extents; i++) {
> +        ret |= bdrv_flush(s->extents[i].file);

This could produce an invalid return code (e.g. ORing together two
different errnos).

I think the best you can do is:
for (...) {
ret = bdrv_flush(s->extents[i].file);
if (ret < 0) {
err = ret;
}
}
return err;

This loses multiple errnos but it doesn't have the potential to corrupt them.

Stefan



Re: [Qemu-devel] [PATCH 10/12] VMDK: change get_cluster_offset return type to success flag

2011-06-18 Thread Stefan Hajnoczi
On Sat, Jun 4, 2011 at 1:43 AM, Fam Zheng  wrote:
> +    if (extent->flat) {
> +               if (m_data) {
> +                       m_data->valid = 0;
> +               }

Why copy-paste this...

> +        *cluster_offset = 0;
> +        return 0;
> +    }
>     if (m_data)
>         m_data->valid = 0;

...instead of moving this above the if statement?

Stefan



Re: [Qemu-devel] [PATCH v5 1/5] Add hard build dependency on glib

2011-06-18 Thread Stefan Hajnoczi
On Sat, Jun 18, 2011 at 5:15 PM, Andreas Färber  wrote:
> With both gcc 4.0.1 and 4.2.1, using CC="gcc -arch ppc64" CPPFLAGS="-arch
> ppc64" and supplying GNU gettext and GNU libiconv (since it chokes on the
> system iconv.h). No Google hit.

Strange, Fink seems to support it:
http://pdb.finkproject.org/pdb/package.php/glib2-dev

Stefan



Re: [Qemu-devel] [PATCH 11/12] VMDK: vmdk_create and options for mono flat image

2011-06-18 Thread Stefan Hajnoczi
On Sat, Jun 4, 2011 at 1:44 AM, Fam Zheng  wrote:
> +    if (flat) {

The flat and !flat cases are too big, please split them out into functions.

> +        const char desc_template[] =
> +        "# Disk DescriptorFile\n"
> +        "version=1\n"
> +        "CID=%x\n"
> +        "parentCID=\n"
> +        "createType=\"monolithicFlat\"\n"
> +        "\n"
> +        "# Extent description\n"
> +        "RW %" PRId64 " FLAT \"%s\" 0\n"
> +        "\n"
> +        "# The Disk Data Base \n"
> +        "#DDB\n"
> +        "\n"
> +        "ddb.virtualHWVersion = \"%d\"\n"
> +        "ddb.geometry.cylinders = \"%" PRId64 "\"\n"
> +        "ddb.geometry.heads = \"16\"\n"
> +        "ddb.geometry.sectors = \"63\"\n"
> +        "ddb.adapterType = \"ide\"\n";

This is almost identical to the desc_template[] below.  Please use
createType=%s and extent type format specifiers to share this template
instead of copy-pasting it.

> +        char ext_filename[1024];
> +        strncpy(ext_filename, filename, 1024);
> +        ext_filename[1023] = '\0';
> +        if (backing_file) {
> +            /* not supporting backing file for flat image */
> +            return -1;
> +        }
> +        if (!strcmp(&ext_filename[strlen(ext_filename) - 5], ".vmdk"))
> +            strcpy(&ext_filename[strlen(ext_filename) - 5], "-flat.vmdk");
> +        else
> +            strcat(ext_filename, "-flat.vmdk");
> +        /* create extent first */
> +        fd = open(
> +                ext_filename,
> +                O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
> +                0644);
> +        if (fd < 0)
> +            return -errno;
> +        ret = ftruncate(fd, total_size * 512);
> +        if (ret) goto exit;
> +        close(fd);
> +
> +        /* generate descriptor file */
> +        snprintf(desc, sizeof(desc), desc_template, (unsigned int)time(NULL),
> +                 total_size, ext_filename,
> +                 (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
> +                 total_size / (int64_t)(63 * 16));
> +        fd = open(
> +                filename,
> +                O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
> +                0644);
> +        if (fd < 0)
> +            return -errno;
> +        ret = qemu_write_full(fd, desc, strlen(desc));
> +        if (ret != strlen(desc)) {
> +            ret = -errno;
> +            goto exit;
> +        }
> +        ret = 0;
> +    } else {
> +        const char desc_template[] =
> +        "# Disk DescriptorFile\n"
> +        "version=1\n"
> +        "CID=%x\n"
> +        "parentCID=\n"
> +        "createType=\"monolithicSparse\"\n"
> +        "\n"
> +        "# Extent description\n"
> +        "RW %" PRId64 " SPARSE \"%s\"\n"
> +        "\n"
> +        "# The Disk Data Base \n"
> +        "#DDB\n"
> +        "\n"
> +        "ddb.virtualHWVersion = \"%d\"\n"
> +        "ddb.geometry.cylinders = \"%" PRId64 "\"\n"
> +        "ddb.geometry.heads = \"16\"\n"
> +        "ddb.geometry.sectors = \"63\"\n"
> +        "ddb.adapterType = \"ide\"\n";
> +        /* XXX: add support for backing file */

Is this comment still true?  It seems there is support for a backing
file via vmdk_snapshot_create().

Stefan



Re: [Qemu-devel] [PATCH 12/12] Add disk_size field to BlockDriverState structure

2011-06-18 Thread Stefan Hajnoczi
On Sat, Jun 4, 2011 at 1:44 AM, Fam Zheng  wrote:
> The `qemu-img info` results for mono flat image are no longer
> accurate, as the "disk size" was the length of bs->file, which is not
> the case for multi file images (such as vmdk images with multiple
> files).
> The new field disk_size in BlockDriverState can be used by block
> driver to tell the exact disk size of block image (only valid if the
> field is set to non-zero).
>
> Signed-off-by: Fam Zheng 
> ---
>  block/vmdk.c |    1 +
>  block_int.h  |    1 +
>  qemu-img.c   |    6 +-
>  3 files changed, 7 insertions(+), 1 deletions(-)

Instead of introducing a variable please add a new BlockDriver
.bdrv_get_allocated_file_size() function pointer and move
get_allocated_file_size() from qemu-img.c into block/raw-posix.c and
block/raw-win32.c.

qemu-img.c should call bdrv_get_allocated_file_size() and
bdrv_get_allocated_file_size() could look like this:

int bdrv_get_allocated_file_size(BlockDriverState *bs, int64_t *size)
{
BlockDriver *drv = bs->drv;

if (!drv) {
return -ENOMEDIUM;
}

if (drv->bdrv_get_allocated_file_size) {
return drv->bdrv_get_allocated_file_size(bs, size);
}

if (bs->file) {
return bdrv_get_allocated_file_size(bs->file, size);
}
return -ENOTSUP;
}

> @@ -607,6 +607,7 @@ static int vmdk_open_desc_file(BlockDriverState
> *bs, int flags)
>     extent = s->extents;
>     vmdk_parse_extents(buf, s->extents, bs->file->filename);
>     bs->total_sectors = extent->sectors;
> +    bs->disk_size = bdrv_getlength(bs->file) + bdrv_getlength(extent->file);

This should call bdrv_get_allocated_file_size() so add together the
actually allocated amount, not what bdrv_getlength() returns.

Stefan



Re: [Qemu-devel] [PATCH 0/12] Adding VMDK monolithic flat support

2011-06-18 Thread Stefan Hajnoczi
On Sat, Jun 4, 2011 at 1:39 AM, Fam Zheng  wrote:
> This is a split patch series of the previous monolithic flat support
> version, with improvements according to Kevin's comments.

Thanks for these patches!  I have left review comments for this series.

Eventually you will touch most of the vmdk code so I think using QEMU
coding style, making parsing robust, and using -errno return values
instead of just 0/-1 should be done.  The existing code doesn't do
these things so it will be a little extra work but worth it in order
to make vmdk a more modern, first-class block driver in QEMU.

Stefan



Re: [Qemu-devel] [PATCH v5 1/5] Add hard build dependency on glib

2011-06-18 Thread Andreas Färber

Am 18.06.2011 um 19:21 schrieb Stefan Hajnoczi:

On Sat, Jun 18, 2011 at 5:15 PM, Andreas Färber > wrote:
With both gcc 4.0.1 and 4.2.1, using CC="gcc -arch ppc64"  
CPPFLAGS="-arch
ppc64" and supplying GNU gettext and GNU libiconv (since it chokes  
on the

system iconv.h). No Google hit.


Strange, Fink seems to support it:
http://pdb.finkproject.org/pdb/package.php/glib2-dev


Says nothing about powerpc64...

Andreas


Re: [Qemu-devel] [RFC v2 23/23] 40p: Add an IBM 8514/A graphics card

2011-06-18 Thread Blue Swirl
On Thu, Jun 16, 2011 at 3:02 AM, Andreas Färber  wrote:
> The IBM E15 is equivalent to an S3 Vision864.
>
> Lacking S3 SDAC (86C716) support, the DAC indizes are translated
> to greyscale colors. This works sufficiently to observe firmware
> boot progress.
>
> Cc: Hervé Poussineau 
>
> Fixed off-by-one drawing issue.
> Replaced hardcoded color for RECT.
> Separate I/O debug output for readability.
> Start cleaning up the naming s3 vs. ibm8514.
> Prepare support for DAC_MASK, DAC_R_INDEX, DAC_W_INDEX, DAC_DATA regs.
>
> Cc: Roy Tam 
> Signed-off-by: Andreas Färber 
> ---
>  Makefile.objs                   |    1 +
>  default-configs/ppc-softmmu.mak |    1 +
>  hw/pci_ids.h                    |    3 +
>  hw/ppc_prep.c                   |    2 +
>  hw/vga-ibm8514.c                |  780 
> +++
>  5 files changed, 787 insertions(+), 0 deletions(-)
>  create mode 100644 hw/vga-ibm8514.c
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 7c5..95dcd91 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -270,6 +270,7 @@ hw-obj-y += qdev-addr.o
>  hw-obj-$(CONFIG_VGA_PCI) += vga-pci.o
>  hw-obj-$(CONFIG_VGA_ISA) += vga-isa.o
>  hw-obj-$(CONFIG_VGA_ISA_MM) += vga-isa-mm.o
> +hw-obj-$(CONFIG_VGA_IBM8514) += vga-ibm8514.o
>  hw-obj-$(CONFIG_VMWARE_VGA) += vmware_vga.o
>  hw-obj-$(CONFIG_VMMOUSE) += vmmouse.o
>
> diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
> index 303929f..f9c97b7 100644
> --- a/default-configs/ppc-softmmu.mak
> +++ b/default-configs/ppc-softmmu.mak
> @@ -6,6 +6,7 @@ CONFIG_ISA_MMIO=y
>  CONFIG_ESCC=y
>  CONFIG_M48T59=y
>  CONFIG_VGA_PCI=y
> +CONFIG_VGA_IBM8514=y
>  CONFIG_SERIAL=y
>  CONFIG_PARALLEL=y
>  CONFIG_I8254=y
> diff --git a/hw/pci_ids.h b/hw/pci_ids.h
> index d3bef0e..821421c 100644
> --- a/hw/pci_ids.h
> +++ b/hw/pci_ids.h
> @@ -97,6 +97,9 @@
>  #define PCI_VENDOR_ID_FREESCALE          0x1957
>  #define PCI_DEVICE_ID_MPC8533E           0x0030
>
> +#define PCI_VENDOR_ID_S3                 0x5333
> +#define PCI_DEVICE_ID_S3_864             0x88c0
> +
>  #define PCI_VENDOR_ID_INTEL              0x8086
>  #define PCI_DEVICE_ID_INTEL_82378        0x0484
>  #define PCI_DEVICE_ID_INTEL_82441        0x1237
> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
> index 6ae1635..c215b0f 100644
> --- a/hw/ppc_prep.c
> +++ b/hw/ppc_prep.c
> @@ -747,6 +747,8 @@ static void ibm_40p_init(ram_addr_t ram_size,
>     qdev_prop_set_uint8(&isa->qdev, "board-identification", 0xfc);
>     qdev_init_nofail(&isa->qdev);
>
> +    pci_create_simple(pci_bus, PCI_DEVFN(2, 0), "s3-vision864");
> +
>     /* Super I/O (parallel + serial ports) */
>     isa = isa_create("isa-pc87312");
>     qdev_prop_set_chr(&isa->qdev, "parallel", parallel_hds[0]);
> diff --git a/hw/vga-ibm8514.c b/hw/vga-ibm8514.c
> new file mode 100644
> index 000..a87afe1
> --- /dev/null
> +++ b/hw/vga-ibm8514.c
> @@ -0,0 +1,780 @@
> +/*
> + * QEMU PCI IBM 8514/A Emulator.
> + *
> + * Copyright (c) 2010 Hervé Poussineau
> + * Copyright (c) 2010-2011 Andreas Färber
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +/* Documentation available at
> + * http://www.datasheetarchive.com/Indexer/Datasheet-06/DSA0091551.html
> + */
> +
> +#include "console.h"
> +#include "pci.h"
> +#include "vga_int.h"
> +#include "pixel_ops.h"
> +
> +//#define DEBUG_8514
> +//#define DEBUG_8514_IO
> +
> +#ifdef DEBUG_8514
> +#define DPRINTF(fmt, ...) \
> +do { printf("8514: " fmt , ## __VA_ARGS__); } while (0)
> +#else
> +#define DPRINTF(fmt, ...) do {} while (0)
> +#endif
> +#ifdef DEBUG_8514_IO
> +#define DPRINTF_IO(fmt, ...) \
> +do { printf("8514: " fmt , ## __VA_ARGS__); } while (0)
> +#else
> +#define DPRINTF_IO(fmt, ...) do {} while (0)
> +#endif
> +#define BADF(fmt, ...) \
> +do { fprintf(stderr, "8514 ERROR: " fmt , ## __VA_ARGS__);} while (0)
> +
> +enum {
> +    REG_CMD = 0x9AE8,
> +    REG_PIX_TRANS = 0x

Re: [Qemu-devel] [PATCH v5 1/5] Add hard build dependency on glib

2011-06-18 Thread Stefan Hajnoczi
On Sat, Jun 18, 2011 at 8:44 PM, Andreas Färber  wrote:
> Am 18.06.2011 um 19:21 schrieb Stefan Hajnoczi:
>
>> On Sat, Jun 18, 2011 at 5:15 PM, Andreas Färber 
>> wrote:
>>>
>>> With both gcc 4.0.1 and 4.2.1, using CC="gcc -arch ppc64" CPPFLAGS="-arch
>>> ppc64" and supplying GNU gettext and GNU libiconv (since it chokes on the
>>> system iconv.h). No Google hit.
>>
>> Strange, Fink seems to support it:
>> http://pdb.finkproject.org/pdb/package.php/glib2-dev
>
> Says nothing about powerpc64...

Shows my ignorance of Mac or Darwin :).  I figured 10.5/powerpc is
what you're after.

Stefan



Re: [Qemu-devel] [PATCH v2] Add support for fd: protocol

2011-06-18 Thread Blue Swirl
On Thu, Jun 16, 2011 at 5:48 PM, Corey Bryant  wrote:
>
>
> On 06/15/2011 03:12 PM, Blue Swirl wrote:
>>
>> On Tue, Jun 14, 2011 at 4:31 PM, Corey Bryant  wrote:
>>>
>>> >  sVirt provides SELinux MAC isolation for Qemu guest processes and
>>> > their
>>> >  corresponding resources (image files). sVirt provides this support
>>> >  by labeling guests and resources with security labels that are stored
>>> >  in file system extended attributes. Some file systems, such as NFS, do
>>> >  not support the extended attribute security namespace, which is needed
>>> >  for image file isolation when using the sVirt SELinux security driver
>>> >  in libvirt.
>>> >
>>> >  The proposed solution entails a combination of Qemu, libvirt, and
>>> >  SELinux patches that work together to isolate multiple guests' images
>>> >  when they're stored in the same NFS mount. This results in an
>>> >  environment where sVirt isolation and NFS image file isolation can
>>> > both
>>> >  be provided.
>>> >
>>> >  This patch contains the Qemu code to support this solution. I would
>>> >  like to solicit input from the libvirt community prior to starting
>>> >  the libvirt patch.
>>> >
>>> >  Currently, Qemu opens an image file in addition to performing the
>>> >  necessary read and write operations. The proposed solution will move
>>> >  the open out of Qemu and into libvirt. Once libvirt opens an image
>>> >  file for the guest, it will pass the file descriptor to Qemu via a
>>> >  new fd: protocol.
>>> >
>>> >  If the image file resides in an NFS mount, the following SELinux
>>> > policy
>>> >  changes will provide image isolation:
>>> >
>>> >    - A new SELinux boolean is created (e.g. virt_read_write_nfs) to
>>> >      allow Qemu (svirt_t) to only have SELinux read and write
>>> >      permissions on nfs_t files
>>> >
>>> >    - Qemu (svirt_t) also gets SELinux use permissions on libvirt
>>> >      (virtd_t) file descriptors
>>> >
>>> >  Following is a sample invocation of Qemu using the fd: protocol on
>>> >  the command line:
>>> >
>>> >      qemu -drive file=fd:4,format=qcow2
>>> >
>>> >  The fd: protocol is also supported by the drive_add monitor command.
>>> >  This requires that the specified file descriptor is passed to the
>>> >  monitor alongside a prior getfd monitor command.
>>> >
>>> >  There are some additional features provided by certain image types
>>> >  where Qemu reopens the image file. All of these scenarios will be
>>> >  unsupported for the fd: protocol, at least for this patch:
>>> >
>>> >    - The -snapshot command line option
>>> >    - The savevm monitor command
>>> >    - The snapshot_blkdev monitor command
>>> >    - Starting Qemu with a backing file
>>
>> There's also native CDROM device. Did you consider adding an explicit
>> reopen method to block layer?
>
> Thanks. Yes it looks like I overlooked CDROM reopens.
>
> I'm not sure that I'm clear on the purpose of the reopen function. Would the
> goal be to funnel all block layer reopens through a single function,
> enabling potential future support where a privileged layer of Qemu, or
> libvirt, performs the open?

Eventually yes, but I think it would help also now by moving the
checks to a single place. It's a bit orthogonal to this patch though.

>>> >  The thought is that this support can be added in the future, but is
>>> >  not required for the initial fd: support.
>>> >
>>> >  This patch was tested with the following formats: raw, cow, qcow,
>>> >  qcow2, qed, and vmdk, using the fd: protocol from the command line
>>> >  and the monitor. Tests were also run to verify existing file name
>>> >  support and qemu-img were not regressed. Non-valid file descriptors,
>>> >  fd: without format, snapshot and backing files were also tested.
>>> >
>>> >  Signed-off-by: Corey Bryant
>>> >  ---
>>> >    block.c           |   16 ++
>>> >    block.h           |    1 +
>>> >    block/cow.c       |    5 +++
>>> >    block/qcow.c      |    5 +++
>>> >    block/qcow2.c     |    5 +++
>>> >    block/qed.c       |    4 ++
>>> >    block/raw-posix.c |   81
>>> > +++--
>>> >    block/vmdk.c      |    5 +++
>>> >    block_int.h       |    1 +
>>> >    blockdev.c        |   10 ++
>>> >    monitor.c         |    5 +++
>>> >    monitor.h         |    1 +
>>> >    qemu-options.hx   |    8 +++--
>>> >    qemu-tool.c       |    5 +++
>>> >    14 files changed, 140 insertions(+), 12 deletions(-)
>>> >
>>> >  diff --git a/block.c b/block.c
>>> >  index 24a25d5..500db84 100644
>>> >  --- a/block.c
>>> >  +++ b/block.c
>>> >  @@ -536,6 +536,10 @@ int bdrv_open(BlockDriverState *bs, const char
>>> > *filename, int flags,
>>> >           char tmp_filename[PATH_MAX];
>>> >           char backing_filename[PATH_MAX];
>>> >
>>> >  +        if (bdrv_is_fd_protocol(bs)) {
>>> >  +            return -ENOTSUP;
>>> >  +        }
>>> >  +
>>> >           /* if snapshot, we create a temporary backing file and open
>>> > it
>>> >          

Re: [Qemu-devel] [PATCH v5 1/5] Add hard build dependency on glib

2011-06-18 Thread Andreas Färber

Am 18.06.2011 um 22:46 schrieb Stefan Hajnoczi:

On Sat, Jun 18, 2011 at 8:44 PM, Andreas Färber > wrote:

Am 18.06.2011 um 19:21 schrieb Stefan Hajnoczi:

On Sat, Jun 18, 2011 at 5:15 PM, Andreas Färber >

wrote:


With both gcc 4.0.1 and 4.2.1, using CC="gcc -arch ppc64"  
CPPFLAGS="-arch
ppc64" and supplying GNU gettext and GNU libiconv (since it  
chokes on the

system iconv.h). No Google hit.


Strange, Fink seems to support it:
http://pdb.finkproject.org/pdb/package.php/glib2-dev


Says nothing about powerpc64...


Shows my ignorance of Mac or Darwin :).  I figured 10.5/powerpc is
what you're after.


No prob, it was a very short-lived technology - arrived in v10.4 half- 
done, became usable in v10.5 and was dropped in v10.6 in favor of Intel.

You have the same ppc vs. ppc64 split for Linux packages btw.

GLib 2.22.4 finished building okay now. I've filed a bug against GLib.  
Hope it works at runtime.


Andreas


Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot

2011-06-18 Thread Blue Swirl
On Sat, Jun 18, 2011 at 6:42 AM, Amit Shah  wrote:
> On (Fri) 17 Jun 2011 [15:08:11], Luiz Capitulino wrote:
>
>> > >          if (!cpkt.value) {
>> > > -            error_report("virtio-serial-bus: Guest failure in adding 
>> > > device %s\n",
>> > > -                         vser->bus.qbus.name);
>> > > -            break;
>> > > +            error_report("virtio-serial-bus: Guest failure in adding 
>> > > device %s\n", vser->bus.qbus.name);
>> > > +            return;
>> >
>> > The line split should remain -- else it goes beyond 80 chars.
>>
>> It's already beyond 80 chars to me.
>
> I prefer to not break strings that get printed out -- makes it easier
> for greppers to find out the source of the string.

Please read CODING_STYLE and use scripts/checkpatch.pl before
submitting patches.

>> > > @@ -346,8 +339,13 @@ static void handle_control_message(VirtIOSerial 
>> > > *vser, void *buf, size_t len)
>> > >          QTAILQ_FOREACH(port, &vser->ports, next) {
>> > >              send_control_event(port, VIRTIO_CONSOLE_PORT_ADD, 1);
>> > >          }
>> > > -        break;
>> > > +        return;
>> > > +    }
>> >
>> > Makes me think of one case (totally unrelated to what you found)where
>> > the guest can fool us: by sending multiple VIRTIO_CONSOLE_DEVICE_READY
>> > messages.
>>
>> It will be handled just fine, no?
>
> We'll send out the VIRTIO_CONSOLE_PORT_ADD events for each port
> (again).  That's the case now.  No idea how the code might change in
> the future and we could end up doing something else in addition which
> might be bad.  Anyway, all this is for a buggy or a bad guest.
>
>                Amit
>
>



[Qemu-devel] [PATCH] Sparc32: dummy implementation of MXCC MMU breakpoint registers

2011-06-18 Thread Blue Swirl
Add dummy registers for SuperSPARC MXCC MMU counter breakpoints.

Signed-off-by: Blue Swirl 
---
 target-sparc/cpu.h   |1 +
 target-sparc/op_helper.c |   26 --
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 320530e..b5d5291 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -403,6 +403,7 @@ typedef struct CPUSPARCState {
 uint32_t mmuregs[32];
 uint64_t mxccdata[4];
 uint64_t mxccregs[8];
+uint32_t mmubpctrv, mmubpctrc, mmubpctrs, mmubpaction;
 uint64_t mmubpregs[4];
 uint64_t prom_addr;
 #endif
diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index b38691e..e9cc1f5 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -1940,7 +1940,6 @@ uint64_t helper_ld_asi(target_ulong addr, int
asi, int size, int sign)
 case 0x31: // Turbosparc RAM snoop
 case 0x32: // Turbosparc page table descriptor diagnostic
 case 0x39: /* data cache diagnostic register */
-case 0x4c: /* SuperSPARC MMU Breakpoint Action register */
 ret = 0;
 break;
 case 0x38: /* SuperSPARC MMU Breakpoint Control Registers */
@@ -1966,6 +1965,18 @@ uint64_t helper_ld_asi(target_ulong addr, int
asi, int size, int sign)
 ret);
 }
 break;
+case 0x49: /* SuperSPARC MMU Counter Breakpoint Value */
+ret = env->mmubpctrv;
+break;
+case 0x4a: /* SuperSPARC MMU Counter Breakpoint Control */
+ret = env->mmubpctrc;
+break;
+case 0x4b: /* SuperSPARC MMU Counter Breakpoint Status */
+ret = env->mmubpctrs;
+break;
+case 0x4c: /* SuperSPARC MMU Breakpoint Action */
+ret = env->mmubpaction;
+break;
 case 8: /* User code access, XXX */
 default:
 do_unassigned_access(addr, 0, 0, asi, size);
@@ -2304,7 +2315,6 @@ void helper_st_asi(target_ulong addr, uint64_t
val, int asi, int size)
// descriptor diagnostic
 case 0x36: /* I-cache flash clear */
 case 0x37: /* D-cache flash clear */
-case 0x4c: /* breakpoint action */
 break;
 case 0x38: /* SuperSPARC MMU Breakpoint Control Registers*/
 {
@@ -2328,6 +2338,18 @@ void helper_st_asi(target_ulong addr, uint64_t
val, int asi, int size)
 env->mmuregs[reg]);
 }
 break;
+case 0x49: /* SuperSPARC MMU Counter Breakpoint Value */
+env->mmubpctrv = val & 0x;
+break;
+case 0x4a: /* SuperSPARC MMU Counter Breakpoint Control */
+env->mmubpctrc = val & 0x3;
+break;
+case 0x4b: /* SuperSPARC MMU Counter Breakpoint Status */
+env->mmubpctrs = val & 0x3;
+break;
+case 0x4c: /* SuperSPARC MMU Breakpoint Action */
+env->mmubpaction = val & 0x1fff;
+break;
 case 8: /* User code access, XXX */
 case 9: /* Supervisor code access, XXX */
 default:
-- 
1.6.2.4
From 52463bf611b3353842525cf7544d7b3d55da8a1e Mon Sep 17 00:00:00 2001
Message-Id: <52463bf611b3353842525cf7544d7b3d55da8a1e.1308433438.git.blauwir...@gmail.com>
From: Blue Swirl 
Date: Sat, 18 Jun 2011 20:27:05 +
Subject: [PATCH] Sparc32: dummy implementation of MXCC MMU breakpoint registers

Add dummy registers for SuperSPARC MXCC MMU counter breakpoints.

Signed-off-by: Blue Swirl 
---
 target-sparc/cpu.h   |1 +
 target-sparc/op_helper.c |   26 --
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 320530e..b5d5291 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -403,6 +403,7 @@ typedef struct CPUSPARCState {
 uint32_t mmuregs[32];
 uint64_t mxccdata[4];
 uint64_t mxccregs[8];
+uint32_t mmubpctrv, mmubpctrc, mmubpctrs, mmubpaction;
 uint64_t mmubpregs[4];
 uint64_t prom_addr;
 #endif
diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index b38691e..e9cc1f5 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -1940,7 +1940,6 @@ uint64_t helper_ld_asi(target_ulong addr, int asi, int size, int sign)
 case 0x31: // Turbosparc RAM snoop
 case 0x32: // Turbosparc page table descriptor diagnostic
 case 0x39: /* data cache diagnostic register */
-case 0x4c: /* SuperSPARC MMU Breakpoint Action register */
 ret = 0;
 break;
 case 0x38: /* SuperSPARC MMU Breakpoint Control Registers */
@@ -1966,6 +1965,18 @@ uint64_t helper_ld_asi(target_ulong addr, int asi, int size, int sign)
 ret);
 }
 break;
+case 0x49: /* SuperSPARC MMU Counter Breakpoint Value */
+ret = env->mmubpctrv;
+break;
+case 0x4a: /* SuperSPARC MMU Counter Breakpoint Control */
+ret = env->mmubpctrc;
+break;
+case 0x4b: /* SuperSPARC MMU Counter Breakpoint Status */
+ret = env->mmubpctrs;
+break;
+case 0x4c: /* SuperS

Re: [Qemu-devel] [PATCH] Sparc32: dummy implementation of MXCC MMU breakpoint registers

2011-06-18 Thread Peter Maydell
On 18 June 2011 22:45, Blue Swirl  wrote:
> Add dummy registers for SuperSPARC MXCC MMU counter breakpoints.
>
> Signed-off-by: Blue Swirl 

> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
> index 320530e..b5d5291 100644
> --- a/target-sparc/cpu.h
> +++ b/target-sparc/cpu.h
> @@ -403,6 +403,7 @@ typedef struct CPUSPARCState {
>     uint32_t mmuregs[32];
>     uint64_t mxccdata[4];
>     uint64_t mxccregs[8];
> +    uint32_t mmubpctrv, mmubpctrc, mmubpctrs, mmubpaction;
>     uint64_t mmubpregs[4];
>     uint64_t prom_addr;
>  #endif

Shouldn't there be a corresponding change to target-sparc/machine.c
adding these to cpu_save()/cpu_load() ?  [the existing mxccdata,
mxccregs, mmubpregs don't seem to be saved/restored either...]

-- PMM



Re: [Qemu-devel] [PATCH] Sparc32: dummy implementation of MXCC MMU breakpoint registers

2011-06-18 Thread Robert Reif

Blue Swirl wrote:

Add dummy registers for SuperSPARC MXCC MMU counter breakpoints.

Signed-off-by: Blue Swirl
---
  target-sparc/cpu.h   |1 +
  target-sparc/op_helper.c |   26 --
  2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 320530e..b5d5291 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -403,6 +403,7 @@ typedef struct CPUSPARCState {
  uint32_t mmuregs[32];
  uint64_t mxccdata[4];
  uint64_t mxccregs[8];
+uint32_t mmubpctrv, mmubpctrc, mmubpctrs, mmubpaction;
  uint64_t mmubpregs[4];
  uint64_t prom_addr;
  #endif
diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index b38691e..e9cc1f5 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -1940,7 +1940,6 @@ uint64_t helper_ld_asi(target_ulong addr, int
asi, int size, int sign)
  case 0x31: // Turbosparc RAM snoop
  case 0x32: // Turbosparc page table descriptor diagnostic
  case 0x39: /* data cache diagnostic register */
-case 0x4c: /* SuperSPARC MMU Breakpoint Action register */
  ret = 0;
  break;
  case 0x38: /* SuperSPARC MMU Breakpoint Control Registers */
@@ -1966,6 +1965,18 @@ uint64_t helper_ld_asi(target_ulong addr, int
asi, int size, int sign)
  ret);
  }
  break;
+case 0x49: /* SuperSPARC MMU Counter Breakpoint Value */
+ret = env->mmubpctrv;
+break;
+case 0x4a: /* SuperSPARC MMU Counter Breakpoint Control */
+ret = env->mmubpctrc;
+break;
+case 0x4b: /* SuperSPARC MMU Counter Breakpoint Status */
+ret = env->mmubpctrs;
+break;
+case 0x4c: /* SuperSPARC MMU Breakpoint Action */
+ret = env->mmubpaction;
+break;
  case 8: /* User code access, XXX */
  default:
  do_unassigned_access(addr, 0, 0, asi, size);
@@ -2304,7 +2315,6 @@ void helper_st_asi(target_ulong addr, uint64_t
val, int asi, int size)
 // descriptor diagnostic
  case 0x36: /* I-cache flash clear */
  case 0x37: /* D-cache flash clear */
-case 0x4c: /* breakpoint action */
  break;
  case 0x38: /* SuperSPARC MMU Breakpoint Control Registers*/
  {
@@ -2328,6 +2338,18 @@ void helper_st_asi(target_ulong addr, uint64_t
val, int asi, int size)
  env->mmuregs[reg]);
  }
  break;
+case 0x49: /* SuperSPARC MMU Counter Breakpoint Value */
+env->mmubpctrv = val&  0x;
+break;
+case 0x4a: /* SuperSPARC MMU Counter Breakpoint Control */
+env->mmubpctrc = val&  0x3;
+break;
+case 0x4b: /* SuperSPARC MMU Counter Breakpoint Status */
+env->mmubpctrs = val&  0x3;
+break;
+case 0x4c: /* SuperSPARC MMU Breakpoint Action */
+env->mmubpaction = val&  0x1fff;
+break;
  case 8: /* User code access, XXX */
  case 9: /* Supervisor code access, XXX */
  default:
   

The breakpoint action register is 64 bits wide.



[Qemu-devel] Bonjour cher Ami

2011-06-18 Thread maurice . chabert
Bonjour
Comment vous aller j espère que vous aller bien

A Votre Honneur,
Je sais que ce message t'apparaîtra comme une surprise puisse qu'on ne ce 
connais pas mais la grâce de Dieu m'a dirigé vers toi et je voudrais que tu lis 
attentivement et sois bénis au nom de Jésus. Je suis Monsieur MAURICE CHABERT 
ressortissant Canadien né en 1942 à Montréal et maintenant résidant à COTONOU.

Ma famille et moi avions aménagés par faute de moyens depuis tout petit j'ai 
perdu mes parents à l'âge de 30ans ma femme est décédée je n’ai pas eu d'enfant 
avec elle mon frère aîné a été tué avec sa petite famille en 2002 lors de la 
guerre en Côte d'ivoire puisse qu’il vivait dans ce pays avec sa famille, et 
voilà que je me retrouve aujourd'hui sans enfants et sans personnes.
Je suis sous contrôle médical depuis plus de deux mois ici a Cotonou car j'ai 
un cancer de la gorge, je souffre terriblement en ce moment mon médecin 
traitant vient de m'informer que mes jours sont comptés du fait de mon état de 
santé dégradante donc condamne à une mort certaine.

Actuellement, j'ai épuisé toutes mes épargnes pour mes soins médicaux mais je 
dispose néanmoins de fonds destinés à mon projet d'œuvres caritatives, ces 
fonds sont sur un compte classé/fixe et bloqué au sein d'une banque locale.
Pour toute opérations financières sur ces fonds il me faut d'abord les 
débloquer le déblocage direct de ces fonds entraîne immédiatement des taxes et 
les impôts exigés par le fisc béninoise qui sont vraiment énorme ce qui 
réduirait environ de moitié le capital
ma situation matrimoniale est telle que je suis veuf du fait que j'ai perdus ma 
femme depuis plus de 10ans maintenant et nous n'avons malheureusement pas eu 
d'enfant ensemble ce qui fais que je n'ai personne à qui léguer mon héritage.

C’est pourquoi, pour débloquer mes fonds que je voudrais procéder par une 
donation de façon à ce que il n'y a pas de taxe élevée sur mes fonds
Pour ce fait je voudrais dans le souci d'aider les démunis et crée un 
orphelinat en ma mémoire raison pour laquelle je te donne ce dit héritage 
s'élevant à une valeur de millions d'euro 2500.000€ Euro pour te permettre 
d'établir une fondation de bienfaisance en ma mémoire afin que la grâce de Dieu 
soit avec moi jusqu'à ma dernière demeure pour que je puisse bénéficier d'une 
place honorable auprès du Seigneur notre père.

Je n’ai aucune crainte car avant de te contacter j'ai prié pendant plusieurs 
nuits pour que le seigneur Jésus Christ puisse m'accorder le contact d'une 
personne de confiance à qui je pourrai confier cette affaire et c'est à la 
suite de cela que j'ai fais des recherches qui m'ont permis d'avoir ton adresse
Sache que tu peut conserver la moitié de cet argent pour toi et le reste 
servira à crée une fondation de bienfaisance en ma mémoire ainsi qu'une 
fédération de lutte contre le cancer et construis aussi des orphelinats
Je voudrais avoir les informations suivantes ton nom et prénom, ton adresse 
précise et ton contact téléphonique permanent afin de les transmettre à mon 
notaire pour qu'ensemble vous effectuiez les démarches de transaction
Email: maurice.chab...@hotmail.fr

La Bible dit que chaque chose à son temps et il est temps pour toi de recevoir 
ces fonds pour les œuvres de l'éternel. Demande et tu recevras, cherche et tu 
trouveras, frappe et l'on t'ouvrira. Mathieu 7:7.
Je compte sur ta bonne volonté et surtout le bon usage de ces fonds.

Dans l'attente de tes nouvelles, reçois mes cordiales et fraternelles
Salutations.
Sincèrement ton dévoue et que Dieu te Bénisse.
Maurice  Chabert