Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Stefan Hajnoczi]

On Thu, Feb 28, 2013 at 03:24:27PM +0000, Dietmar Maurer wrote:
> > > Unfortunately, NBD add considerable overheads. I guess the socket
> > communications copies data.
> > > This is really unnecessary if I can write directly to the output stream.
> > 
> > The disk is the bottleneck, not memory bandwidth.  Hard disks only do
> > 10-100 MB/sec and SSDs only do a couple 100 MB/sec.  Memory copy is
> > insignificant compared to the I/O activity required to copy out the entire disk
> > image, not to mention delaying guest writes until we read the original data from
> > the disk.
> > 
> > Unless there's a concrete performance problem here this is premature
> > optimization.
> 
> I currently test with about 700MB read speed, and get a slow down by factor 1.7.
> So memory copy is very significant , or there is something wrong with nbd.c.

What are the details of the test?

Is it using 64 KB writes and have you tried 256 KB writes?

Stefan

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Dietmar Maurer]

> What are the details of the test?
> 
> Is it using 64 KB writes and have you tried 256 KB writes?

I use a modified 'qemu-img convert' at 64KB block size (I need 64KB for backup).

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Kevin Wolf]

Am 04.03.2013 um 14:16 hat Dietmar Maurer geschrieben:
> > What are the details of the test?
> > 
> > Is it using 64 KB writes and have you tried 256 KB writes?
> 
> I use a modified 'qemu-img convert' at 64KB block size (I need 64KB for backup).

Maybe you'd better use a different output format that doesn't restrict
you to 64k writes.

Kevin

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Dietmar Maurer]

> > > Is it using 64 KB writes and have you tried 256 KB writes?
> >
> > I use a modified 'qemu-img convert' at 64KB block size (I need 64KB for
> backup).
> 
> Maybe you'd better use a different output format that doesn't restrict you to
> 64k writes.

The output format is not really the restriction. The problem is that an additional
IPC layer add overhead, an d I do not want that (because it is totally unnecessary).

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Stefan Hajnoczi]

On Mon, Mar 04, 2013 at 02:33:16PM +0000, Dietmar Maurer wrote:
> > > > Is it using 64 KB writes and have you tried 256 KB writes?
> > >
> > > I use a modified 'qemu-img convert' at 64KB block size (I need 64KB for
> > backup).
> > 
> > Maybe you'd better use a different output format that doesn't restrict you to
> > 64k writes.
> 
> The output format is not really the restriction. The problem is that an additional
> IPC layer add overhead, an d I do not want that (because it is totally unnecessary).

I missed the reason why you cannot increase the block size.  At least
increase the NBD write size - the VMA process can break up a large write
however it likes.  Do this and the overhead is divided by 4.

Stefan

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Dietmar Maurer]

> > > Maybe you'd better use a different output format that doesn't
> > > restrict you to 64k writes.
> >
> > The output format is not really the restriction. The problem is that
> > an additional IPC layer add overhead, an d I do not want that (because it is
> totally unnecessary).
> 
> I missed the reason why you cannot increase the block size.  

When we run backup, we need to read such block on every write from the guest.
So if we increase block size we get additional delays.

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Kevin Wolf]

Am 06.03.2013 um 15:42 hat Dietmar Maurer geschrieben:
> > > > Maybe you'd better use a different output format that doesn't
> > > > restrict you to 64k writes.
> > >
> > > The output format is not really the restriction. The problem is that
> > > an additional IPC layer add overhead, an d I do not want that (because it is
> > totally unnecessary).
> > 
> > I missed the reason why you cannot increase the block size.  
> 
> When we run backup, we need to read such block on every write from the guest.
> So if we increase block size we get additional delays.

How about variable block sizes? I mean this is a stream format that has
a header for each block anyway. Include a size there and be done.

Kevin

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Dietmar Maurer]

> > When we run backup, we need to read such block on every write from the
> guest.
> > So if we increase block size we get additional delays.
> 
> How about variable block sizes? I mean this is a stream format that has a header
> for each block anyway. Include a size there and be done.

You can make that as complex as you want. I simply do not need that.

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Kevin Wolf]

Am 06.03.2013 um 16:33 hat Dietmar Maurer geschrieben:
> > > When we run backup, we need to read such block on every write from the
> > guest.
> > > So if we increase block size we get additional delays.
> > 
> > How about variable block sizes? I mean this is a stream format that has a header
> > for each block anyway. Include a size there and be done.
> 
> You can make that as complex as you want. I simply do not need that.

Then you also don't need the performance that you lose by using NBD.

Kevin

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Dietmar Maurer]

> > > How about variable block sizes? I mean this is a stream format that
> > > has a header for each block anyway. Include a size there and be done.
> >
> > You can make that as complex as you want. I simply do not need that.
> 
> Then you also don't need the performance that you lose by using NBD.

Why exactly?

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Kevin Wolf]

Am 06.03.2013 um 18:39 hat Dietmar Maurer geschrieben:
> > > > How about variable block sizes? I mean this is a stream format that
> > > > has a header for each block anyway. Include a size there and be done.
> > >
> > > You can make that as complex as you want. I simply do not need that.
> > 
> > Then you also don't need the performance that you lose by using NBD.
> 
> Why exactly?

Because your format kills more performance than any NBD connection
could.

Kevin

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Dietmar Maurer]

> > > > You can make that as complex as you want. I simply do not need that.
> > >
> > > Then you also don't need the performance that you lose by using NBD.
> >
> > Why exactly?
> 
> Because your format kills more performance than any NBD connection could.

That is not true.

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Stefan Hajnoczi]

On Wed, Mar 06, 2013 at 02:42:57PM +0000, Dietmar Maurer wrote:
> > > > Maybe you'd better use a different output format that doesn't
> > > > restrict you to 64k writes.
> > >
> > > The output format is not really the restriction. The problem is that
> > > an additional IPC layer add overhead, an d I do not want that (because it is
> > totally unnecessary).
> > 
> > I missed the reason why you cannot increase the block size.  
> 
> When we run backup, we need to read such block on every write from the guest.
> So if we increase block size we get additional delays.

Don't increase the bitmap block size.

Just let the block job do larger reads.  This is the bulk of the I/O
workload.  You can use large reads here independently of the bitmap
block size.

That way guest writes still only have a 64 KB read overhead but you
reduce the overhead of doing so many 64 KB writes from the backup block
job.

Stefan

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Dietmar Maurer]

> > When we run backup, we need to read such block on every write from the
> guest.
> > So if we increase block size we get additional delays.
> 
> Don't increase the bitmap block size.
> 
> Just let the block job do larger reads.  This is the bulk of the I/O workload.  You
> can use large reads here independently of the bitmap block size.
> 
> That way guest writes still only have a 64 KB read overhead but you reduce the
> overhead of doing so many 64 KB writes from the backup block job.

If there are many writes from the guest, you still get many 64KB block.

Anyways, and additional RPC layer always adds overhead, and It can be completely avoided.
Maybe not much, but I want to make backup as efficient as possible.

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Stefan Hajnoczi]

On Thu, Mar 07, 2013 at 09:28:40AM +0000, Dietmar Maurer wrote:
> > > When we run backup, we need to read such block on every write from the
> > guest.
> > > So if we increase block size we get additional delays.
> > 
> > Don't increase the bitmap block size.
> > 
> > Just let the block job do larger reads.  This is the bulk of the I/O workload.  You
> > can use large reads here independently of the bitmap block size.
> > 
> > That way guest writes still only have a 64 KB read overhead but you reduce the
> > overhead of doing so many 64 KB writes from the backup block job.
> 
> If there are many writes from the guest, you still get many 64KB block.

In the common case the backup block job does significantly more I/O than
the guest, unless your workload is dd if=/dev/vda of=/dev/null.
Remember the block job is reading the *entire* disk!

> Anyways, and additional RPC layer always adds overhead, and It can be completely avoided.
> Maybe not much, but I want to make backup as efficient as possible.

The drawbacks outweight the performance advantage:

1. QEMU can neither backup nor restore without help from the management
   tool.  This is a strong indicator that the backup archive code should
   live outside QEMU.  I doesn't make sense for proxmox, oVirt,
   OpenStack, and others to each maintain their backup archive code
   inside qemu.git, tied to QEMU's C codebase, release cycle, and
   license.

2. QEMU already has interfaces to export the vmstate and block device
   snapshots: migration/savevm and BlockDriverState (NBD for IPC or
   raw/qcow2/vmdk for file).  It is not necessary to add a
   special-purpose interface just for backup.

3. The backup block job can be composed together with other QMP commands
   to achieve scenarios besides just VMA backup.  It's more flexible to
   add simple primitives that can be combined instead of adding a
   monolithic backup command.

For these reasons, I'm against putting backup archive code into QEMU.

If performance is key, please look into incremental backups.  Taking a
full copy of the disk image every time affects guest performance much
more than IPC overhead.  Plus there'll be less IPC if only dirty blocks
are backed up :).

Stefan

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Dietmar Maurer]

> > Anyways, and additional RPC layer always adds overhead, and It can be
> completely avoided.
> > Maybe not much, but I want to make backup as efficient as possible.
> 
> The drawbacks outweight the performance advantage:

not for me.

> 1. QEMU can neither backup nor restore without help from the management
>    tool.  

Backup works perfectly with the current patches. You can easily trigger a backup using 
a HMP command. This is not really important, but works.

> This is a strong indicator that the backup archive code should
>    live outside QEMU.  I doesn't make sense for proxmox, oVirt,
>    OpenStack, and others to each maintain their backup archive code
>    inside qemu.git, tied to QEMU's C codebase, release cycle, and
>    license.
> 2. QEMU already has interfaces to export the vmstate and block device
>    snapshots: migration/savevm and BlockDriverState (NBD for IPC or
>    raw/qcow2/vmdk for file).  It is not necessary to add a
>    special-purpose interface just for backup.
> 
> 3. The backup block job can be composed together with other QMP commands
>    to achieve scenarios besides just VMA backup.  It's more flexible to
>    add simple primitives that can be combined instead of adding a
>    monolithic backup command.
> 
> For these reasons, I'm against putting backup archive code into QEMU.

That is OK for me - I already maintain the code outside of qemu.

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Stefan Hajnoczi]

On Fri, Mar 8, 2013 at 12:01 PM, Dietmar Maurer <dietmar@proxmox.com> wrote:
>> > Anyways, and additional RPC layer always adds overhead, and It can be
>> completely avoided.
>> > Maybe not much, but I want to make backup as efficient as possible.
>>
>> The drawbacks outweight the performance advantage:
>
> not for me.
>
>> 1. QEMU can neither backup nor restore without help from the management
>>    tool.
>
> Backup works perfectly with the current patches. You can easily trigger a backup using
> a HMP command. This is not really important, but works.

If you send me a VMA file I can't restore it without knowing your
command-line and possibly setting up the environment.  QEMU just can't
do that, it's out of scope.  That is why the backup archive is a
management tool concept.

>> This is a strong indicator that the backup archive code should
>>    live outside QEMU.  I doesn't make sense for proxmox, oVirt,
>>    OpenStack, and others to each maintain their backup archive code
>>    inside qemu.git, tied to QEMU's C codebase, release cycle, and
>>    license.
>> 2. QEMU already has interfaces to export the vmstate and block device
>>    snapshots: migration/savevm and BlockDriverState (NBD for IPC or
>>    raw/qcow2/vmdk for file).  It is not necessary to add a
>>    special-purpose interface just for backup.
>>
>> 3. The backup block job can be composed together with other QMP commands
>>    to achieve scenarios besides just VMA backup.  It's more flexible to
>>    add simple primitives that can be combined instead of adding a
>>    monolithic backup command.
>>
>> For these reasons, I'm against putting backup archive code into QEMU.
>
> That is OK for me - I already maintain the code outside of qemu.

Does this mean you will keep this patch series out-of-tree?

What I am looking for is a stripped down patch series with just a
backup block job (no backup archive writer or migration code).  That
would be easily merged and saves you front rebasing this series as
QEMU changes.

Stefan

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Dietmar Maurer]

> >> 1. QEMU can neither backup nor restore without help from the management
> >>    tool.
> >
> > Backup works perfectly with the current patches. You can easily
> > trigger a backup using a HMP command. This is not really important, but works.
> 
> If you send me a VMA file I can't restore it without knowing your command-line
> and possibly setting up the environment.  QEMU just can't do that, it's out of
> scope.  That is why the backup archive is a management tool concept.

I don't really know why you insist on that claim. I can do a backup with hmp command,
and then restore that later using the management tools. I just need to issue the
correct hmp backup command.

But this is not relevant for this discussion, so let us continue. We talked about the overhead
of the additional IPC layer.

> 
> >> This is a strong indicator that the backup archive code should
> >>    live outside QEMU.  I doesn't make sense for proxmox, oVirt,
> >>    OpenStack, and others to each maintain their backup archive code
> >>    inside qemu.git, tied to QEMU's C codebase, release cycle, and
> >>    license.
> >> 2. QEMU already has interfaces to export the vmstate and block device
> >>    snapshots: migration/savevm and BlockDriverState (NBD for IPC or
> >>    raw/qcow2/vmdk for file).  It is not necessary to add a
> >>    special-purpose interface just for backup.
> >>
> >> 3. The backup block job can be composed together with other QMP
> commands
> >>    to achieve scenarios besides just VMA backup.  It's more flexible to
> >>    add simple primitives that can be combined instead of adding a
> >>    monolithic backup command.
> >>
> >> For these reasons, I'm against putting backup archive code into QEMU.
> >
> > That is OK for me - I already maintain the code outside of qemu.
> 
> Does this mean you will keep this patch series out-of-tree?
> 
> What I am looking for is a stripped down patch series with just a backup block
> job (no backup archive writer or migration code).  That would be easily merged
> and saves you front rebasing this series as QEMU changes.

That is Patch 2/6?

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Stefan Hajnoczi]

On Fri, Mar 8, 2013 at 6:44 PM, Dietmar Maurer <dietmar@proxmox.com> wrote:
>> >> This is a strong indicator that the backup archive code should
>> >>    live outside QEMU.  I doesn't make sense for proxmox, oVirt,
>> >>    OpenStack, and others to each maintain their backup archive code
>> >>    inside qemu.git, tied to QEMU's C codebase, release cycle, and
>> >>    license.
>> >> 2. QEMU already has interfaces to export the vmstate and block device
>> >>    snapshots: migration/savevm and BlockDriverState (NBD for IPC or
>> >>    raw/qcow2/vmdk for file).  It is not necessary to add a
>> >>    special-purpose interface just for backup.
>> >>
>> >> 3. The backup block job can be composed together with other QMP
>> commands
>> >>    to achieve scenarios besides just VMA backup.  It's more flexible to
>> >>    add simple primitives that can be combined instead of adding a
>> >>    monolithic backup command.
>> >>
>> >> For these reasons, I'm against putting backup archive code into QEMU.
>> >
>> > That is OK for me - I already maintain the code outside of qemu.
>>
>> Does this mean you will keep this patch series out-of-tree?
>>
>> What I am looking for is a stripped down patch series with just a backup block
>> job (no backup archive writer or migration code).  That would be easily merged
>> and saves you front rebasing this series as QEMU changes.
>
> That is Patch 2/6?

Yes.  I sent an RFC series that shows this approach.  It is just a
prototype but it demonstrates that the NBD approach works and performs
reasonably well for a quick Python hack.

The stripped down patch series needs:
1. (Optional) query-block virtual_size field if management tool needs
to query drive size
2. Patch 2/6 with review comments addressed
3. 'block-backup' QMP command (and optionally HMP command)

That's it!

Stefan

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Dietmar Maurer]

> >> What I am looking for is a stripped down patch series with just a
> >> backup block job (no backup archive writer or migration code).  That
> >> would be easily merged and saves you front rebasing this series as QEMU
> changes.
> >
> > That is Patch 2/6?
> 
> Yes.  I sent an RFC series that shows this approach.  It is just a prototype but it
> demonstrates that the NBD approach works and performs reasonably well for a
> quick Python hack.
> 
> The stripped down patch series needs:
> 1. (Optional) query-block virtual_size field if management tool needs to query
> drive size 2. Patch 2/6 with review comments addressed 3. 'block-backup' QMP
> command (and optionally HMP command)
> 
> That's it!

Thanks. I also have an  implementation here using that approach (using nbd.c).
The problem is the performance you get. 28% delay is not acceptable.

In fact, I want to have the fastest possible solution (additional delay = 0).

You want to move the code out of qemu, so that management frameworks
can easily plug their own backup tools. That is perfectly valid, but we should do
it in a way that does not influence performance.

So are there any other ideas?

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Stefan Hajnoczi]

On Sun, Mar 10, 2013 at 10:32 AM, Dietmar Maurer <dietmar@proxmox.com> wrote:
>> >> What I am looking for is a stripped down patch series with just a
>> >> backup block job (no backup archive writer or migration code).  That
>> >> would be easily merged and saves you front rebasing this series as QEMU
>> changes.
>> >
>> > That is Patch 2/6?
>>
>> Yes.  I sent an RFC series that shows this approach.  It is just a prototype but it
>> demonstrates that the NBD approach works and performs reasonably well for a
>> quick Python hack.
>>
>> The stripped down patch series needs:
>> 1. (Optional) query-block virtual_size field if management tool needs to query
>> drive size 2. Patch 2/6 with review comments addressed 3. 'block-backup' QMP
>> command (and optionally HMP command)
>>
>> That's it!
>
> Thanks. I also have an  implementation here using that approach (using nbd.c).
> The problem is the performance you get. 28% delay is not acceptable.
>
> In fact, I want to have the fastest possible solution (additional delay = 0).
>
> You want to move the code out of qemu, so that management frameworks
> can easily plug their own backup tools. That is perfectly valid, but we should do
> it in a way that does not influence performance.
>
> So are there any other ideas?

Same discussion in the other thread, so let's move there where we both
just posted.

Stefan

Re: [Qemu-devel] [PATCH v4 3/6] add backup related monitor commands

[Dietmar Maurer]

> >> For these reasons, I'm against putting backup archive code into QEMU.
> >
> > That is OK for me - I already maintain the code outside of qemu.
> 
> Does this mean you will keep this patch series out-of-tree?

You are 'against putting backup archive code into QEMU', so I need to
maintain it out-of-tree.