Message ID | 1361875228-15769-10-git-send-email-xiawenc@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes: > Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > --- > block.c | 24 ++++++++++++++++++++++++ > include/block/block.h | 2 ++ > savevm.c | 22 ---------------------- > 3 files changed, 26 insertions(+), 22 deletions(-) Perhaps savevm.c isn't the best home for snapshot stuff, but I don't like it in block.c any better. block.c is already unwieldy, and I'd like us to try limiting it to core block stuff. Kevin, Stefan, any ideas?
Am 27.02.2013 um 17:04 hat Markus Armbruster geschrieben: > Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes: > > > Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> > > Reviewed-by: Eric Blake <eblake@redhat.com> > > --- > > block.c | 24 ++++++++++++++++++++++++ > > include/block/block.h | 2 ++ > > savevm.c | 22 ---------------------- > > 3 files changed, 26 insertions(+), 22 deletions(-) > > Perhaps savevm.c isn't the best home for snapshot stuff, but I don't > like it in block.c any better. block.c is already unwieldy, and I'd > like us to try limiting it to core block stuff. > > Kevin, Stefan, any ideas? Take some more snapshot related functions from block.c and move them to block/snapshot.c? Kevin
于 2013-2-28 0:22, Kevin Wolf 写道: > Am 27.02.2013 um 17:04 hat Markus Armbruster geschrieben: >> Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes: >> >>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> >>> Reviewed-by: Eric Blake <eblake@redhat.com> >>> --- >>> block.c | 24 ++++++++++++++++++++++++ >>> include/block/block.h | 2 ++ >>> savevm.c | 22 ---------------------- >>> 3 files changed, 26 insertions(+), 22 deletions(-) >> >> Perhaps savevm.c isn't the best home for snapshot stuff, but I don't >> like it in block.c any better. block.c is already unwieldy, and I'd >> like us to try limiting it to core block stuff. >> >> Kevin, Stefan, any ideas? > > Take some more snapshot related functions from block.c and move them to > block/snapshot.c? > > Kevin > Hi, Stefan Do you also agree about adding new file block/snapshot.c?
于 2013-3-1 9:51, Wenchao Xia 写道: > 于 2013-2-28 0:22, Kevin Wolf 写道: >> Am 27.02.2013 um 17:04 hat Markus Armbruster geschrieben: >>> Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes: >>> >>>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> >>>> Reviewed-by: Eric Blake <eblake@redhat.com> >>>> --- >>>> block.c | 24 ++++++++++++++++++++++++ >>>> include/block/block.h | 2 ++ >>>> savevm.c | 22 ---------------------- >>>> 3 files changed, 26 insertions(+), 22 deletions(-) >>> >>> Perhaps savevm.c isn't the best home for snapshot stuff, but I don't >>> like it in block.c any better. block.c is already unwieldy, and I'd >>> like us to try limiting it to core block stuff. >>> >>> Kevin, Stefan, any ideas? >> >> Take some more snapshot related functions from block.c and move them to >> block/snapshot.c? >> >> Kevin >> > Hi, Stefan > Do you also agree about adding new file block/snapshot.c? > > Hi, I am going to add include/block/snapshot.h and block/snapshot.c, all internal snapshot function in block.c will be moved there. external backing file related function will not be touched and remain in block.c. This consume many effort and once done it will be hard to revert back to form an incremental commit in this serials, so before coding, please give your opinion if u don't agree about it.
On Fri, Mar 01, 2013 at 09:51:53AM +0800, Wenchao Xia wrote: > 于 2013-2-28 0:22, Kevin Wolf 写道: > >Am 27.02.2013 um 17:04 hat Markus Armbruster geschrieben: > >>Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes: > >> > >>>Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> > >>>Reviewed-by: Eric Blake <eblake@redhat.com> > >>>--- > >>> block.c | 24 ++++++++++++++++++++++++ > >>> include/block/block.h | 2 ++ > >>> savevm.c | 22 ---------------------- > >>> 3 files changed, 26 insertions(+), 22 deletions(-) > >> > >>Perhaps savevm.c isn't the best home for snapshot stuff, but I don't > >>like it in block.c any better. block.c is already unwieldy, and I'd > >>like us to try limiting it to core block stuff. > >> > >>Kevin, Stefan, any ideas? > > > >Take some more snapshot related functions from block.c and move them to > >block/snapshot.c? > > > >Kevin > > > Hi, Stefan > Do you also agree about adding new file block/snapshot.c? Yes, I agree. Stefan
于 2013-3-4 21:02, Stefan Hajnoczi 写道: > On Fri, Mar 01, 2013 at 09:51:53AM +0800, Wenchao Xia wrote: > 于 2013-2-28 0:22, Kevin Wolf 写道: >>> Am 27.02.2013 um 17:04 hat Markus Armbruster geschrieben: >>>> Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes: >>>> >>>>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> >>>>> Reviewed-by: Eric Blake <eblake@redhat.com> >>>>> --- >>>>> block.c | 24 ++++++++++++++++++++++++ >>>>> include/block/block.h | 2 ++ >>>>> savevm.c | 22 ---------------------- >>>>> 3 files changed, 26 insertions(+), 22 deletions(-) >>>> >>>> Perhaps savevm.c isn't the best home for snapshot stuff, but I don't >>>> like it in block.c any better. block.c is already unwieldy, and I'd >>>> like us to try limiting it to core block stuff. >>>> >>>> Kevin, Stefan, any ideas? >>> >>> Take some more snapshot related functions from block.c and move them to >>> block/snapshot.c? >>> >>> Kevin >>> >> Hi, Stefan >> Do you also agree about adding new file block/snapshot.c? > > Yes, I agree. > > Stefan > great. Also there are some other functions such as image info collecting, do you think new file block/misc.c is suitable to store those functions there?
On Tue, Mar 05, 2013 at 03:11:48PM +0800, Wenchao Xia wrote: > 于 2013-3-4 21:02, Stefan Hajnoczi 写道: > >On Fri, Mar 01, 2013 at 09:51:53AM +0800, Wenchao Xia wrote: > 于 2013-2-28 0:22, Kevin Wolf 写道: > >>>Am 27.02.2013 um 17:04 hat Markus Armbruster geschrieben: > >>>>Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes: > >>>> > >>>>>Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com> > >>>>>Reviewed-by: Eric Blake <eblake@redhat.com> > >>>>>--- > >>>>> block.c | 24 ++++++++++++++++++++++++ > >>>>> include/block/block.h | 2 ++ > >>>>> savevm.c | 22 ---------------------- > >>>>> 3 files changed, 26 insertions(+), 22 deletions(-) > >>>> > >>>>Perhaps savevm.c isn't the best home for snapshot stuff, but I don't > >>>>like it in block.c any better. block.c is already unwieldy, and I'd > >>>>like us to try limiting it to core block stuff. > >>>> > >>>>Kevin, Stefan, any ideas? > >>> > >>>Take some more snapshot related functions from block.c and move them to > >>>block/snapshot.c? > >>> > >>>Kevin > >>> > >>Hi, Stefan > >> Do you also agree about adding new file block/snapshot.c? > > > >Yes, I agree. > > > >Stefan > > > great. Also there are some other functions such as image info > collecting, do you think new file block/misc.c is suitable > to store those functions there? As discussed on IRC, it's fine by me. If a better way to organize these functions becomes clear in the future they can be moved. Stefan
Am 05.03.2013 um 10:21 hat Stefan Hajnoczi geschrieben: > On Tue, Mar 05, 2013 at 03:11:48PM +0800, Wenchao Xia wrote: > > 于 2013-3-4 21:02, Stefan Hajnoczi 写道: > > >On Fri, Mar 01, 2013 at 09:51:53AM +0800, Wenchao Xia wrote: > 于 2013-2-28 0:22, Kevin Wolf 写道: > > >>Hi, Stefan > > >> Do you also agree about adding new file block/snapshot.c? > > > > > >Yes, I agree. > > > > > >Stefan > > > > > great. Also there are some other functions such as image info > > collecting, do you think new file block/misc.c is suitable > > to store those functions there? > > As discussed on IRC, it's fine by me. If a better way to organize these > functions becomes clear in the future they can be moved. As also discussed on IRC, I'm not excited by having a block/misc.c. We already have something for "everything block related that doesn't fit elsewhere" and it's block.c. I couldn't tell if a function belong into block.c or block/misc.c. I suggested having a file that concentrates all function related to QAPI, the monitor and JSON (including the qemu-img JSON output) and call it something like block/qapi.c (I'm open for better suggestions). This would at least make it clear if a function should be in there or not. Kevin
Il 05/03/2013 11:08, Kevin Wolf ha scritto: >> > >> > As discussed on IRC, it's fine by me. If a better way to organize these >> > functions becomes clear in the future they can be moved. > As also discussed on IRC, I'm not excited by having a block/misc.c. We > already have something for "everything block related that doesn't fit > elsewhere" and it's block.c. I couldn't tell if a function belong into > block.c or block/misc.c. > > I suggested having a file that concentrates all function related to > QAPI, the monitor and JSON (including the qemu-img JSON output) and call > it something like block/qapi.c (I'm open for better suggestions). This > would at least make it clear if a function should be in there or not. I agree with Kevin. Paolo
diff --git a/block.c b/block.c index 3976167..a97c7ec 100644 --- a/block.c +++ b/block.c @@ -3421,6 +3421,30 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs, return -ENOTSUP; } +int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, + const char *name) +{ + QEMUSnapshotInfo *sn_tab, *sn; + int nb_sns, i, ret; + + ret = -ENOENT; + nb_sns = bdrv_snapshot_list(bs, &sn_tab); + if (nb_sns < 0) { + return ret; + } + + for (i = 0; i < nb_sns; i++) { + sn = &sn_tab[i]; + if (!strcmp(sn->id_str, name) || !strcmp(sn->name, name)) { + *sn_info = *sn; + ret = 0; + break; + } + } + g_free(sn_tab); + return ret; +} + /* backing_file can either be relative, or absolute, or a protocol. If it is * relative, it must be relative to the chain. So, passing in bs->filename * from a BDS as backing_file should not be done, as that may be relative to diff --git a/include/block/block.h b/include/block/block.h index a820184..d2b8bd1 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -343,6 +343,8 @@ int bdrv_snapshot_list(BlockDriverState *bs, int bdrv_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_name); char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn); +int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, + const char *name); char *get_human_readable_size(char *buf, int buf_size, int64_t size); int path_is_absolute(const char *path); diff --git a/savevm.c b/savevm.c index a8a53ef..f73ac32 100644 --- a/savevm.c +++ b/savevm.c @@ -2060,28 +2060,6 @@ out: return ret; } -static int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, - const char *name) -{ - QEMUSnapshotInfo *sn_tab, *sn; - int nb_sns, i, ret; - - ret = -ENOENT; - nb_sns = bdrv_snapshot_list(bs, &sn_tab); - if (nb_sns < 0) - return ret; - for(i = 0; i < nb_sns; i++) { - sn = &sn_tab[i]; - if (!strcmp(sn->id_str, name) || !strcmp(sn->name, name)) { - *sn_info = *sn; - ret = 0; - break; - } - } - g_free(sn_tab); - return ret; -} - /* * Deletes snapshots of a given name in all opened images. */