For drive-backup and blockdev-backup, expose the manual-cull
property, having it default to false. There are no universal
creation parameters, so it must be added to each job type that
it makes sense for individually.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 blockdev.c           | 10 ++++++++--
 qapi/block-core.json | 21 ++++++++++++++++-----
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index ee07bca..ba2ebfb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3198,6 +3198,9 @@ static BlockJob *do_drive_backup(DriveBackup *backup, BlockJobTxn *txn,
     if (!backup->has_job_id) {
         backup->job_id = NULL;
     }
+    if (!backup->has_manual_cull) {
+        backup->manual_cull = false;
+    }
     if (!backup->has_compress) {
         backup->compress = false;
     }
@@ -3290,7 +3293,7 @@ static BlockJob *do_drive_backup(DriveBackup *backup, BlockJobTxn *txn,
         }
     }
 
-    job = backup_job_create(backup->job_id, false, bs, target_bs,
+    job = backup_job_create(backup->job_id, backup->manual_cull, bs, target_bs,
                             backup->speed, backup->sync, bmap, backup->compress,
                             backup->on_source_error, backup->on_target_error,
                             BLOCK_JOB_DEFAULT, NULL, NULL, txn, &local_err);
@@ -3341,6 +3344,9 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn,
     if (!backup->has_job_id) {
         backup->job_id = NULL;
     }
+    if (!backup->has_manual_cull) {
+        backup->manual_cull = false;
+    }
     if (!backup->has_compress) {
         backup->compress = false;
     }
@@ -3369,7 +3375,7 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn,
             goto out;
         }
     }
-    job = backup_job_create(backup->job_id, false, bs, target_bs,
+    job = backup_job_create(backup->job_id, backup->manual_cull, bs, target_bs,
                             backup->speed, backup->sync, NULL, backup->compress,
                             backup->on_source_error, backup->on_target_error,
                             BLOCK_JOB_DEFAULT, NULL, NULL, txn, &local_err);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index de322d1..c646743 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1104,6 +1104,11 @@
 # @job-id: identifier for the newly-created block job. If
 #          omitted, the device name will be used. (Since 2.7)
 #
+# @manual-cull: Whether or not the job created by this command needs to be
+#               cleaned up manually via block-job-cull or not. The default is
+#               false. When true, the job will remain in a "completed" state
+#               until culled manually with block-job-cull. (Since 2.11)
+#
 # @device: the device name or node-name of a root node which should be copied.
 #
 # @target: the target of the new image. If the file exists, or if it
@@ -1144,9 +1149,10 @@
 # Since: 1.6
 ##
 { 'struct': 'DriveBackup',
-  'data': { '*job-id': 'str', 'device': 'str', 'target': 'str',
-            '*format': 'str', 'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
-            '*speed': 'int', '*bitmap': 'str', '*compress': 'bool',
+  'data': { '*job-id': 'str', '*manual-cull': 'bool', 'device': 'str',
+            'target': 'str', '*format': 'str', 'sync': 'MirrorSyncMode',
+            '*mode': 'NewImageMode', '*speed': 'int', '*bitmap': 'str',
+            '*compress': 'bool',
             '*on-source-error': 'BlockdevOnError',
             '*on-target-error': 'BlockdevOnError' } }
 
@@ -1156,6 +1162,11 @@
 # @job-id: identifier for the newly-created block job. If
 #          omitted, the device name will be used. (Since 2.7)
 #
+# @manual-cull: Whether or not the job created by this command needs to be
+#               cleaned up manually via block-job-cull or not. The default is
+#               false. When true, the job will remain in a "completed" state
+#               until culled manually with block-job-cull. (Since 2.11)
+#
 # @device: the device name or node-name of a root node which should be copied.
 #
 # @target: the device name or node-name of the backup target node.
@@ -1185,8 +1196,8 @@
 # Since: 2.3
 ##
 { 'struct': 'BlockdevBackup',
-  'data': { '*job-id': 'str', 'device': 'str', 'target': 'str',
-            'sync': 'MirrorSyncMode',
+  'data': { '*job-id': 'str', '*manual-cull': 'bool', 'device': 'str',
+            'target': 'str', 'sync': 'MirrorSyncMode',
             '*speed': 'int',
             '*compress': 'bool',
             '*on-source-error': 'BlockdevOnError',
-- 
2.9.5

On 03/10/2017 05:15, John Snow wrote:
> For drive-backup and blockdev-backup, expose the manual-cull
> property, having it default to false. There are no universal
> creation parameters, so it must be added to each job type that
> it makes sense for individually.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  blockdev.c           | 10 ++++++++--
>  qapi/block-core.json | 21 ++++++++++++++++-----
>  2 files changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index ee07bca..ba2ebfb 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3198,6 +3198,9 @@ static BlockJob *do_drive_backup(DriveBackup *backup, BlockJobTxn *txn,
>      if (!backup->has_job_id) {
>          backup->job_id = NULL;
>      }
> +    if (!backup->has_manual_cull) {
> +        backup->manual_cull = false;
> +    }
>      if (!backup->has_compress) {
>          backup->compress = false;
>      }
> @@ -3290,7 +3293,7 @@ static BlockJob *do_drive_backup(DriveBackup *backup, BlockJobTxn *txn,
>          }
>      }
>  
> -    job = backup_job_create(backup->job_id, false, bs, target_bs,
> +    job = backup_job_create(backup->job_id, backup->manual_cull, bs, target_bs,
>                              backup->speed, backup->sync, bmap, backup->compress,
>                              backup->on_source_error, backup->on_target_error,
>                              BLOCK_JOB_DEFAULT, NULL, NULL, txn, &local_err);
> @@ -3341,6 +3344,9 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn,
>      if (!backup->has_job_id) {
>          backup->job_id = NULL;
>      }
> +    if (!backup->has_manual_cull) {
> +        backup->manual_cull = false;
> +    }
>      if (!backup->has_compress) {
>          backup->compress = false;
>      }
> @@ -3369,7 +3375,7 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, BlockJobTxn *txn,
>              goto out;
>          }
>      }
> -    job = backup_job_create(backup->job_id, false, bs, target_bs,
> +    job = backup_job_create(backup->job_id, backup->manual_cull, bs, target_bs,
>                              backup->speed, backup->sync, NULL, backup->compress,
>                              backup->on_source_error, backup->on_target_error,
>                              BLOCK_JOB_DEFAULT, NULL, NULL, txn, &local_err);
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index de322d1..c646743 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1104,6 +1104,11 @@
>  # @job-id: identifier for the newly-created block job. If
>  #          omitted, the device name will be used. (Since 2.7)
>  #
> +# @manual-cull: Whether or not the job created by this command needs to be
> +#               cleaned up manually via block-job-cull or not. The default is
> +#               false. When true, the job will remain in a "completed" state
> +#               until culled manually with block-job-cull. (Since 2.11)
> +#
>  # @device: the device name or node-name of a root node which should be copied.
>  #
>  # @target: the target of the new image. If the file exists, or if it
> @@ -1144,9 +1149,10 @@
>  # Since: 1.6
>  ##
>  { 'struct': 'DriveBackup',
> -  'data': { '*job-id': 'str', 'device': 'str', 'target': 'str',
> -            '*format': 'str', 'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
> -            '*speed': 'int', '*bitmap': 'str', '*compress': 'bool',
> +  'data': { '*job-id': 'str', '*manual-cull': 'bool', 'device': 'str',
> +            'target': 'str', '*format': 'str', 'sync': 'MirrorSyncMode',
> +            '*mode': 'NewImageMode', '*speed': 'int', '*bitmap': 'str',
> +            '*compress': 'bool',
>              '*on-source-error': 'BlockdevOnError',
>              '*on-target-error': 'BlockdevOnError' } }
>  
> @@ -1156,6 +1162,11 @@
>  # @job-id: identifier for the newly-created block job. If
>  #          omitted, the device name will be used. (Since 2.7)
>  #
> +# @manual-cull: Whether or not the job created by this command needs to be
> +#               cleaned up manually via block-job-cull or not. The default is
> +#               false. When true, the job will remain in a "completed" state
> +#               until culled manually with block-job-cull. (Since 2.11)

The verb "cull" is a bit weird.  The only alternative that comes to mind
though are "reap" (like processes). There's also "join" (like threads),
but would imply waiting if the jobs hasn't completed yet, and we
probably don't want it.

Paolo

>  # @device: the device name or node-name of a root node which should be copied.
>  #
>  # @target: the device name or node-name of the backup target node.
> @@ -1185,8 +1196,8 @@
>  # Since: 2.3
>  ##
>  { 'struct': 'BlockdevBackup',
> -  'data': { '*job-id': 'str', 'device': 'str', 'target': 'str',
> -            'sync': 'MirrorSyncMode',
> +  'data': { '*job-id': 'str', '*manual-cull': 'bool', 'device': 'str',
> +            'target': 'str', 'sync': 'MirrorSyncMode',
>              '*speed': 'int',
>              '*compress': 'bool',
>              '*on-source-error': 'BlockdevOnError',
> 

On 10/03/2017 11:57 AM, Paolo Bonzini wrote:
> On 03/10/2017 05:15, John Snow wrote:
>> For drive-backup and blockdev-backup, expose the manual-cull
>> property, having it default to false. There are no universal
>> creation parameters, so it must be added to each job type that
>> it makes sense for individually.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>

[...]

> 
> The verb "cull" is a bit weird.  The only alternative that comes to mind
> though are "reap" (like processes). There's also "join" (like threads),
> but would imply waiting if the jobs hasn't completed yet, and we
> probably don't want it.
> 
> Paolo
> 

Sure, open to suggestions. I think Kevin suggested "delete" which I have
reservations about because of people potentially confusing it with
"cancel" or "complete" -- it does not have the capacity to
end/terminate/finish/complete/cancel a job.

"reap" might be fine. I don't really have any strong preference.

--js

On Tue, Oct 03, 2017 at 11:59:28AM -0400, John Snow wrote:
> 
> 
> On 10/03/2017 11:57 AM, Paolo Bonzini wrote:
> > On 03/10/2017 05:15, John Snow wrote:
> >> For drive-backup and blockdev-backup, expose the manual-cull
> >> property, having it default to false. There are no universal
> >> creation parameters, so it must be added to each job type that
> >> it makes sense for individually.
> >>
> >> Signed-off-by: John Snow <jsnow@redhat.com>
> 
> [...]
> 
> > 
> > The verb "cull" is a bit weird.  The only alternative that comes to mind
> > though are "reap" (like processes). There's also "join" (like threads),
> > but would imply waiting if the jobs hasn't completed yet, and we
> > probably don't want it.
> > 
> > Paolo
> > 
> 
> Sure, open to suggestions. I think Kevin suggested "delete" which I have
> reservations about because of people potentially confusing it with
> "cancel" or "complete" -- it does not have the capacity to
> end/terminate/finish/complete/cancel a job.
> 
> "reap" might be fine. I don't really have any strong preference.
>

As far as verbs go, I like both 'reap' and 'delete'.  As far as the
property, naming it 'manual_verb' is a bit odd, too. Maybe a clearer term
for the property would just be 'persistent', with the QMP command being
'block_job_reap' or 'block_job_delete'?

-Jeff

On 10/03/2017 01:43 PM, Jeff Cody wrote:
> On Tue, Oct 03, 2017 at 11:59:28AM -0400, John Snow wrote:
>>
>>
>> On 10/03/2017 11:57 AM, Paolo Bonzini wrote:
>>> On 03/10/2017 05:15, John Snow wrote:
>>>> For drive-backup and blockdev-backup, expose the manual-cull
>>>> property, having it default to false. There are no universal
>>>> creation parameters, so it must be added to each job type that
>>>> it makes sense for individually.
>>>>
>>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>
>> [...]
>>
>>>
>>> The verb "cull" is a bit weird.  The only alternative that comes to mind
>>> though are "reap" (like processes). There's also "join" (like threads),
>>> but would imply waiting if the jobs hasn't completed yet, and we
>>> probably don't want it.
>>>
>>> Paolo
>>>
>>
>> Sure, open to suggestions. I think Kevin suggested "delete" which I have
>> reservations about because of people potentially confusing it with
>> "cancel" or "complete" -- it does not have the capacity to
>> end/terminate/finish/complete/cancel a job.
>>
>> "reap" might be fine. I don't really have any strong preference.
>>
> 
> As far as verbs go, I like both 'reap' and 'delete'.  As far as the
> property, naming it 'manual_verb' is a bit odd, too. Maybe a clearer term
> for the property would just be 'persistent', with the QMP command being
> 'block_job_reap' or 'block_job_delete'?
> 
> -Jeff
> 

As they say, two hard problems in Computer Science ...