From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:46055)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1bv24Y-0002kg-Su
	for qemu-devel@nongnu.org; Fri, 14 Oct 2016 08:58:51 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1bv24W-0002yu-Si
	for qemu-devel@nongnu.org; Fri, 14 Oct 2016 08:58:49 -0400
Date: Fri, 14 Oct 2016 14:58:36 +0200
From: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20161014125836.GC4473@noname.redhat.com>
References: <1476399422-8028-1-git-send-email-jsnow@redhat.com>
	<1476399422-8028-2-git-send-email-jsnow@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1476399422-8028-2-git-send-email-jsnow@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 1/7] blockjobs: hide internal jobs from
 management API
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: John Snow <jsnow@redhat.com>
Cc: qemu-block@nongnu.org, xiecl.fnst@cn.fujitsu.com, wency@cn.fujitsu.com, jcody@redhat.com, qemu-devel@nongnu.org, pbonzini@redhat.com

Am 14.10.2016 um 00:56 hat John Snow geschrieben:
> If jobs are not created directly by the user, do not allow them to be
> seen by the user/management utility. At the moment, 'internal' jobs are
> those that do not have an ID. As of this patch it is impossible to
> create such jobs.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>

block_job_get() still has a strcmp(id, job->id) for all block jobs
without checking job->id != NULL first.

job->id is also used in the error message in block_job_complete(),
though you could argue that we have a bug if this is ever triggered for
internal jobs. Still, there are platform on which this would crash, so
maybe better catch it.

> -BlockJobInfo *block_job_query(BlockJob *job)
> +BlockJobInfo *block_job_query(BlockJob *job, Error **errp)
>  {
> -    BlockJobInfo *info = g_new0(BlockJobInfo, 1);
> +    BlockJobInfo *info;
> +
> +    if (block_job_is_internal(job)) {
> +        error_setg(errp, "Cannot query QEMU internal Jobs");

You definitely have Potential for being a good Student of German. I
agree that a Text is easier to read if you capitalise all Nouns in it,
but I'm afraid this is not Part of the current english Orthography.

> +        return NULL;
> +    }
> +    info = g_new0(BlockJobInfo, 1);
>      info->type      = g_strdup(BlockJobType_lookup[job->driver->job_type]);
>      info->device    = g_strdup(job->id);
>      info->len       = job->len;

Kevin