All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Cc: Kevin Wolf <kwolf@redhat.com>,  Fiona Ebner <f.ebner@proxmox.com>,
	qemu-devel@nongnu.org,  qemu-block@nongnu.org,
	 eblake@redhat.com, hreitz@redhat.com,  jsnow@redhat.com,
	 den@virtuozzo.com, t.lamprecht@proxmox.com,
	 alexander.ivanov@virtuozzo.com
Subject: Re: [PATCH v2 00/10] mirror: allow switching from background to active mode
Date: Thu, 29 Feb 2024 06:28:12 +0100	[thread overview]
Message-ID: <87msrju9eb.fsf@pond.sub.org> (raw)
In-Reply-To: <1310efb0-e211-46f5-b166-d7d529507a43@yandex-team.ru> (Vladimir Sementsov-Ogievskiy's message of "Wed, 28 Feb 2024 21:07:02 +0300")

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> On 03.11.23 18:56, Markus Armbruster wrote:
>> Is the job abstraction a failure?
>>
>> We have
>>
>>      block-job- command      since   job- command    since
>>      -----------------------------------------------------
>>      block-job-set-speed     1.1
>>      block-job-cancel        1.1     job-cancel      3.0
>>      block-job-pause         1.3     job-pause       3.0
>>      block-job-resume        1.3     job-resume      3.0
>>      block-job-complete      1.3     job-complete    3.0
>>      block-job-dismiss       2.12    job-dismiss     3.0
>>      block-job-finalize      2.12    job-finalize    3.0
>>      block-job-change        8.2
>>      query-block-jobs        1.1     query-jobs
>>
>> I was under the impression that we added the (more general) job-
>> commands to replace the (less general) block-job commands, and we're
>> keeping the latter just for compatibility.  Am I mistaken?
>>
>> Which one should be used?
>>
>> Why not deprecate the one that shouldn't be used?
>>
>> The addition of block-job-change without even trying to do job-change
>> makes me wonder: have we given up on the job- interface?
>>
>> I'm okay with giving up on failures.  All I want is clarity.  Right now,
>> I feel thoroughly confused about the status block-jobs and jobs, and how
>> they're related.
>
> Hi! I didn't notice, that the series was finally merged.
>
> About the APIs, I think, of course we should deprecate block-job-* API, because we already have jobs which are not block-jobs, so we can't deprecate job-* API.
>
> So I suggest a plan:
>
> 1. Add job-change command simply in block-core.json, as a simple copy of block-job-change, to not care with resolving inclusion loops. (ha we could simply name our block-job-change to be job-change and place it in block-core.json, but now is too late)
>
> 2. Support changing speed in a new job-chage command. (or both in block-job-change and job-change, keeping them equal)
>
> 3. Deprecate block-job-* APIs
>
> 4. Wait 3 releases
>
> 5. Drop block-job-* APIs
>
> 6. Move all job-related stuff to job.json, drop `{ 'include': 'job.json' }` from block-core.json, and instead include block-core.json into job.json

Sounds good to me.

> If it's OK, I can go through the steps.

Lovely!



  reply	other threads:[~2024-02-29  5:28 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-09  9:46 [PATCH v2 00/10] mirror: allow switching from background to active mode Fiona Ebner
2023-10-09  9:46 ` [PATCH v2 01/10] blockjob: introduce block-job-change QMP command Fiona Ebner
2023-10-10 18:04   ` Vladimir Sementsov-Ogievskiy
2023-10-09  9:46 ` [PATCH v2 02/10] block/mirror: set actively_synced even after the job is ready Fiona Ebner
2023-10-09  9:46 ` [PATCH v2 03/10] block/mirror: move dirty bitmap to filter Fiona Ebner
2023-10-10 19:10   ` Vladimir Sementsov-Ogievskiy
2023-10-09  9:46 ` [PATCH v2 04/10] block/mirror: determine copy_to_target only once Fiona Ebner
2023-10-10 19:23   ` Vladimir Sementsov-Ogievskiy
2023-10-09  9:46 ` [PATCH v2 05/10] mirror: implement mirror_change method Fiona Ebner
2023-10-10 19:37   ` Vladimir Sementsov-Ogievskiy
2023-10-11 11:22     ` Fiona Ebner
2023-10-12 13:54       ` Vladimir Sementsov-Ogievskiy
2023-10-09  9:46 ` [PATCH v2 06/10] qapi/block-core: use JobType for BlockJobInfo's type Fiona Ebner
2023-10-09  9:46 ` [PATCH v2 07/10] qapi/block-core: turn BlockJobInfo into a union Fiona Ebner
2023-10-09  9:46 ` [PATCH v2 08/10] blockjob: query driver-specific info via a new 'query' driver method Fiona Ebner
2023-10-10 19:51   ` Vladimir Sementsov-Ogievskiy
2023-10-09  9:46 ` [PATCH v2 09/10] mirror: return mirror-specific information upon query Fiona Ebner
2023-10-10 19:53   ` Vladimir Sementsov-Ogievskiy
2023-10-09  9:46 ` [PATCH v2 10/10] iotests: adapt test output for new mirror query property Fiona Ebner
2023-10-10 19:57   ` Vladimir Sementsov-Ogievskiy
2023-10-10 17:55 ` [PATCH v2 00/10] mirror: allow switching from background to active mode Vladimir Sementsov-Ogievskiy
2023-10-10 20:01   ` Vladimir Sementsov-Ogievskiy
2023-10-11 10:18   ` Fiona Ebner
2023-10-12 14:10     ` Vladimir Sementsov-Ogievskiy
2023-11-03  9:36       ` Markus Armbruster
2023-11-03 11:54         ` Kevin Wolf
2023-11-03 15:56           ` Markus Armbruster
2024-02-28 18:07             ` Vladimir Sementsov-Ogievskiy
2024-02-29  5:28               ` Markus Armbruster [this message]
2024-03-04 10:48               ` Kevin Wolf
2024-03-04 11:09                 ` Peter Krempa
2024-03-07 19:42                   ` Vladimir Sementsov-Ogievskiy
2024-03-08  8:21                     ` Fiona Ebner
2024-03-08  8:52                     ` Kevin Wolf
2024-03-11 15:15                       ` Vladimir Sementsov-Ogievskiy
2024-03-12 13:44                         ` Vladimir Sementsov-Ogievskiy
2024-03-12 15:49                           ` Kevin Wolf
2024-03-12 18:52                             ` Vladimir Sementsov-Ogievskiy
2024-03-10 21:07                     ` Peter Krempa
2024-03-11 15:51                       ` Vladimir Sementsov-Ogievskiy
2024-03-11 16:07                         ` Peter Krempa
2024-03-04 12:27                 ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87msrju9eb.fsf@pond.sub.org \
    --to=armbru@redhat.com \
    --cc=alexander.ivanov@virtuozzo.com \
    --cc=den@virtuozzo.com \
    --cc=eblake@redhat.com \
    --cc=f.ebner@proxmox.com \
    --cc=hreitz@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=t.lamprecht@proxmox.com \
    --cc=vsementsov@yandex-team.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.