From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55192) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bv6Qw-0004zS-KZ for qemu-devel@nongnu.org; Fri, 14 Oct 2016 13:38:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bv6Qv-0000vl-Ak for qemu-devel@nongnu.org; Fri, 14 Oct 2016 13:38:14 -0400 References: <1476399422-8028-1-git-send-email-jsnow@redhat.com> <1476399422-8028-2-git-send-email-jsnow@redhat.com> <20161014125836.GC4473@noname.redhat.com> From: John Snow Message-ID: <415a4c68-0990-19c3-c49a-fa0d1c99d0e4@redhat.com> Date: Fri, 14 Oct 2016 13:32:17 -0400 MIME-Version: 1.0 In-Reply-To: <20161014125836.GC4473@noname.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/7] blockjobs: hide internal jobs from management API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: xiecl.fnst@cn.fujitsu.com, qemu-block@nongnu.org, jcody@redhat.com, qemu-devel@nongnu.org, pbonzini@redhat.com On 10/14/2016 08:58 AM, Kevin Wolf wrote: > 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 ar= e >> those that do not have an ID. As of this patch it is impossible to >> create such jobs. >> >> Signed-off-by: John Snow > > block_job_get() still has a strcmp(id, job->id) for all block jobs > without checking job->id !=3D NULL first. > Good catch. > 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. > It may be misleading to check for job->id in a function that should not=20 be called when that is false. I will add an assertion instead, with the=20 premise that any potential misuses here must necessarily be internal. >> -BlockJobInfo *block_job_query(BlockJob *job) >> +BlockJobInfo *block_job_query(BlockJob *job, Error **errp) >> { >> - BlockJobInfo *info =3D 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. > ^^^^^^^ This is my favorite email. >> + return NULL; >> + } >> + info =3D g_new0(BlockJobInfo, 1); >> info->type =3D g_strdup(BlockJobType_lookup[job->driver->job= _type]); >> info->device =3D g_strdup(job->id); >> info->len =3D job->len; > > Kevin > --=20 =97js