qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 1/7] blockjobs: hide internal jobs from manageme


From: John Snow
Subject: Re: [Qemu-devel] [PATCH 1/7] blockjobs: hide internal jobs from management API
Date: Fri, 14 Oct 2016 13:32:17 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0



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 are
those that do not have an ID. As of this patch it is impossible to
create such jobs.

Signed-off-by: John Snow <address@hidden>

block_job_get() still has a strcmp(id, job->id) for all block jobs
without checking job->id != 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 be called when that is false. I will add an assertion instead, with the 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 = 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 = 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


--
—js



reply via email to

[Prev in Thread] Current Thread [Next in Thread]